From 6a7ec5c65db02e3595e85ea9d43e629fefdfdb9d Mon Sep 17 00:00:00 2001 From: lyb124553153 <124553153@qq.com> Date: Fri, 18 Nov 2022 15:17:45 +0800 Subject: [PATCH] Skip create project push rule Ship create project push rule when creating project, use predefined push rule instead Changelog: changed EE: true --- ee/app/models/ee/project.rb | 12 +++++ .../ee/merge_requests/merge_base_service.rb | 2 +- ee/app/services/ee/projects/create_service.rb | 2 +- .../inherited_push_rule_for_project.yml | 8 +++ ee/lib/ee/gitlab/checks/base_checker.rb | 2 +- .../ee/gitlab/checks/push_rule_check_spec.rb | 2 + .../checks/push_rules/branch_check_spec.rb | 1 + .../checks/push_rules/commit_check_spec.rb | 1 + .../checks/push_rules/file_size_check_spec.rb | 1 + .../checks/push_rules/tag_check_spec.rb | 1 + ee/spec/models/project_spec.rb | 52 +++++++++++++++++++ ee/spec/models/push_rule_spec.rb | 1 + .../services/projects/create_service_spec.rb | 7 ++- .../push_rules_checks_shared_examples.rb | 14 +++++ 14 files changed, 102 insertions(+), 4 deletions(-) create mode 100644 ee/config/feature_flags/development/inherited_push_rule_for_project.yml diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb index 25e6df7c708578..2bba203ffe497a 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 88eb2be1a9d61c..a338d9a620a0a3 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 ea64122fa59067..d31469d35aeabd 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 00000000000000..d9b74c179c46d4 --- /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 00ab918761066d..b7b94b659ff215 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 5350ccedbbcb3d..fa42a05b798007 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 bb4112aa94a83e..18f6723fa16109 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 c82eb45f824260..40795b1f99138a 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 baa1afd8d37728..32c06195a90499 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 69afd9565341bc..2f23d767d3e2e9 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 445f5af7cb7f8b..1c4be7edd9dfd1 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 093fcbe2ac1231..52b76c315fa668 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 d2d6afde6be7b8..9c8e1d22b720b1 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 ba6349e65681de..f1e5309a411a9b 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 -- GitLab