diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb index 25e6df7c708578e4ed491b9442752dbce52f7fdb..2bba203ffe497ac2dfc5ee842c03c8a6494565d8 100644 --- a/ee/app/models/ee/project.rb +++ b/ee/app/models/ee/project.rb @@ -990,6 +990,18 @@ def only_mirror_protected_branches_column only_mirror_protected_branches end + def predefined_push_rule + return push_rule if ::Feature.disabled?(:inherited_push_rule_for_project, self) + return unless feature_available?(:push_rules) + return push_rule if push_rule + + if group + group.predefined_push_rule + else + PushRule.global + end + end + private def update_legacy_open_source_license_available diff --git a/ee/app/services/ee/merge_requests/merge_base_service.rb b/ee/app/services/ee/merge_requests/merge_base_service.rb index 88eb2be1a9d61c7addb3a50de6b056f3ff0277a2..a338d9a620a0a3f90c787bf010d23a73a5f77114 100644 --- a/ee/app/services/ee/merge_requests/merge_base_service.rb +++ b/ee/app/services/ee/merge_requests/merge_base_service.rb @@ -58,7 +58,7 @@ def hooks_validation_error(merge_request, validate_squash_message: false) def push_rule strong_memoize(:push_rule) do - merge_request.project.push_rule if project.feature_available?(:push_rules) + merge_request.project.predefined_push_rule if project.feature_available?(:push_rules) end end diff --git a/ee/app/services/ee/projects/create_service.rb b/ee/app/services/ee/projects/create_service.rb index ea64122fa590679b0ff9baff779c29317a94cf29..d31469d35aeabd91192f879895511a7652ec6e19 100644 --- a/ee/app/services/ee/projects/create_service.rb +++ b/ee/app/services/ee/projects/create_service.rb @@ -63,7 +63,7 @@ def after_create_actions create_security_policy_configuration_if_exists end - create_predefined_push_rule + create_predefined_push_rule if ::Feature.disabled?(:inherited_push_rule_for_project, project) set_default_compliance_framework end diff --git a/ee/config/feature_flags/development/inherited_push_rule_for_project.yml b/ee/config/feature_flags/development/inherited_push_rule_for_project.yml new file mode 100644 index 0000000000000000000000000000000000000000..d9b74c179c46d4acc9fefbf10a2ce1a965f2a9aa --- /dev/null +++ b/ee/config/feature_flags/development/inherited_push_rule_for_project.yml @@ -0,0 +1,8 @@ +--- +name: inherited_push_rule_for_project +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/104558 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/385375 +milestone: +type: development +group: group::source code +default_enabled: false diff --git a/ee/lib/ee/gitlab/checks/base_checker.rb b/ee/lib/ee/gitlab/checks/base_checker.rb index 00ab918761066da8eea01cf580de71dd395acfe2..b7b94b659ff215e86cee61f7e38759216b455bf2 100644 --- a/ee/lib/ee/gitlab/checks/base_checker.rb +++ b/ee/lib/ee/gitlab/checks/base_checker.rb @@ -11,7 +11,7 @@ module BaseChecker def push_rule strong_memoize(:push_rule) do - project.push_rule + project.predefined_push_rule end end end diff --git a/ee/spec/lib/ee/gitlab/checks/push_rule_check_spec.rb b/ee/spec/lib/ee/gitlab/checks/push_rule_check_spec.rb index 5350ccedbbcb3d5c10fa6d0c57ed632cb1f4df80..fa42a05b79800798ec984122533c5130d7d447b1 100644 --- a/ee/spec/lib/ee/gitlab/checks/push_rule_check_spec.rb +++ b/ee/spec/lib/ee/gitlab/checks/push_rule_check_spec.rb @@ -24,6 +24,8 @@ .to receive(:validate!).and_return(nil) end + it_behaves_like 'use predefined push rules' + it "returns nil on success" do expect(subject.validate!).to be_nil end diff --git a/ee/spec/lib/ee/gitlab/checks/push_rules/branch_check_spec.rb b/ee/spec/lib/ee/gitlab/checks/push_rules/branch_check_spec.rb index bb4112aa94a83eaa5560058ab7f788b6ea67c063..18f6723fa16109239b6157e226f9f38c8a7e34cf 100644 --- a/ee/spec/lib/ee/gitlab/checks/push_rules/branch_check_spec.rb +++ b/ee/spec/lib/ee/gitlab/checks/push_rules/branch_check_spec.rb @@ -10,6 +10,7 @@ let(:ref) { 'refs/heads/a-branch-that-is-not-allowed' } it_behaves_like 'check ignored when push rule unlicensed' + it_behaves_like 'use predefined push rules' it 'rejects the branch that is not allowed' do expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, "Branch name 'a-branch-that-is-not-allowed' does not follow the pattern '^(w*)$'") diff --git a/ee/spec/lib/ee/gitlab/checks/push_rules/commit_check_spec.rb b/ee/spec/lib/ee/gitlab/checks/push_rules/commit_check_spec.rb index c82eb45f824260b09402a7d15a6833b808243ed7..40795b1f99138ae8ba3cfc19fd3c7bd67671af3c 100644 --- a/ee/spec/lib/ee/gitlab/checks/push_rules/commit_check_spec.rb +++ b/ee/spec/lib/ee/gitlab/checks/push_rules/commit_check_spec.rb @@ -10,6 +10,7 @@ let!(:push_rule) { create(:push_rule, :commit_message) } it_behaves_like 'check ignored when push rule unlicensed' + it_behaves_like 'use predefined push rules' it 'returns an error if the rule fails due to missing required characters' do expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, "Commit message does not follow the pattern '#{push_rule.commit_message_regex}'") diff --git a/ee/spec/lib/ee/gitlab/checks/push_rules/file_size_check_spec.rb b/ee/spec/lib/ee/gitlab/checks/push_rules/file_size_check_spec.rb index baa1afd8d37728a481e6ee282dcb7ab17d32c93c..32c06195a90499a080c6cc6ff8ce3bf46b2986ff 100644 --- a/ee/spec/lib/ee/gitlab/checks/push_rules/file_size_check_spec.rb +++ b/ee/spec/lib/ee/gitlab/checks/push_rules/file_size_check_spec.rb @@ -40,6 +40,7 @@ end it_behaves_like 'check ignored when push rule unlicensed' + it_behaves_like 'use predefined push rules' it 'returns an error if file exceeds the maximum file size' do expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, "File \"file.bin\" is larger than the allowed size of 1 MB. Use Git LFS to manage this file.") diff --git a/ee/spec/lib/ee/gitlab/checks/push_rules/tag_check_spec.rb b/ee/spec/lib/ee/gitlab/checks/push_rules/tag_check_spec.rb index 69afd9565341bc308917e0e54f3f7f98686bf4cb..2f23d767d3e2e9507f995a68d7c1bc59cfd6f134 100644 --- a/ee/spec/lib/ee/gitlab/checks/push_rules/tag_check_spec.rb +++ b/ee/spec/lib/ee/gitlab/checks/push_rules/tag_check_spec.rb @@ -12,6 +12,7 @@ let(:ref) { 'refs/tags/v1.0.0' } it_behaves_like 'check ignored when push rule unlicensed' + it_behaves_like 'use predefined push rules' it 'returns an error if the rule denies tag deletion' do expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, 'You cannot delete a tag') diff --git a/ee/spec/models/project_spec.rb b/ee/spec/models/project_spec.rb index 445f5af7cb7f8b58d1e56678c4b811f8f60e8cca..1c4be7edd9dfd1b1c878330d9157e9c12cd1f33f 100644 --- a/ee/spec/models/project_spec.rb +++ b/ee/spec/models/project_spec.rb @@ -799,6 +799,58 @@ end end + describe '#predefined_push_rule' do + subject(:predefined_push_rule) { project.predefined_push_rule } + + context 'when inherited_push_rule_for_project is disabled' do + before do + stub_feature_flags(inherited_push_rule_for_project: false) + end + + it 'return push rule' do + expect(project).to receive(:push_rule) + + subject + end + end + + context 'push rules unlicensed' do + before do + stub_licensed_features(push_rules: false) + end + + it { is_expected.to be_nil } + end + + context 'push rules licensed' do + context 'when has push rule' do + let(:push_rule) { build(:push_rule) } + + before do + project.push_rule = push_rule + end + + it { is_expected.to eq(push_rule) } + end + + context 'when has group push rule' do + let!(:group) { build(:group, push_rule: build(:push_rule)) } + + before do + project.group = group + end + + it { is_expected.to eq(group.push_rule) } + end + + context 'when has global push rule' do + let!(:push_rule_sample) { create(:push_rule_sample) } + + it { is_expected.to eq(push_rule_sample) } + end + end + end + context 'merge requests related settings' do shared_examples 'setting modified by application setting' do where(:feature_enabled, :app_setting, :project_setting, :final_setting) do diff --git a/ee/spec/models/push_rule_spec.rb b/ee/spec/models/push_rule_spec.rb index 093fcbe2ac1231d43ebffd3ea91b13adb08a7c45..52b76c315fa668301ae4e15a52ea522cf869d4c3 100644 --- a/ee/spec/models/push_rule_spec.rb +++ b/ee/spec/models/push_rule_spec.rb @@ -158,6 +158,7 @@ with_them do context "when rule is enabled at global level" do before do + stub_feature_flags(inherited_push_rule_for_project: false) global_push_rule.update_column(setting, value) end diff --git a/ee/spec/services/projects/create_service_spec.rb b/ee/spec/services/projects/create_service_spec.rb index d2d6afde6be7b81ea69d80e5bd15a87b07641dab..9c8e1d22b720b1c2a7e9aa9c8792b60f2c801226 100644 --- a/ee/spec/services/projects/create_service_spec.rb +++ b/ee/spec/services/projects/create_service_spec.rb @@ -199,7 +199,11 @@ end end - context 'push rules' do + context 'when inherited_push_rule_for_project is disabled' do + before do + stub_feature_flags(inherited_push_rule_for_project: false) + end + context 'with sample' do let!(:sample) { create(:push_rule_sample) } @@ -247,6 +251,7 @@ context 'group push rules' do before do stub_licensed_features(push_rules: true) + stub_feature_flags(inherited_push_rule_for_project: false) end context 'project created within a group' do diff --git a/ee/spec/support/shared_examples/lib/gitlab/push_rules_checks_shared_examples.rb b/ee/spec/support/shared_examples/lib/gitlab/push_rules_checks_shared_examples.rb index ba6349e65681debcea1d4377687b1f5d035b10b0..f1e5309a411a9bc2cb1c33291acac7d862406bf7 100644 --- a/ee/spec/support/shared_examples/lib/gitlab/push_rules_checks_shared_examples.rb +++ b/ee/spec/support/shared_examples/lib/gitlab/push_rules_checks_shared_examples.rb @@ -7,3 +7,17 @@ it { is_expected.to be_truthy } end + +RSpec.shared_examples 'use predefined push rules' do + it 'calls Project#predefined_push_rule' do + expect(project).to receive(:predefined_push_rule).and_call_original + + begin + subject.validate! + rescue Gitlab::GitAccess::ForbiddenError + # Do nothing. Rescuing in case subject.validate! raises an error. There + # are consumers of this shared example gsherein subject.validate! will raise + # an error, and some don't. + end + end +end