diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index 657760c69b6149298185f7fb58590170503ad690..3c453a0243e6d663bc6d2903c0d81b9165700579 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -923,7 +923,6 @@ Use `detailed_merge_status` instead of `merge_status` to account for all potenti - `preparing`: Merge request diff is being created. - `requested_changes`: The merge request has reviewers who have requested changes. - `security_policy_violations`: All security policies must be satisfied. - Requires the `policy_mergability_check` feature flag to be enabled. - `status_checks_must_pass`: All status checks must pass before merge. - `unchecked`: Git has not yet tested if a valid merge is possible. - `locked_paths`: Paths locked by other users must be unlocked before merging to default branch. diff --git a/ee/app/models/ee/merge_request.rb b/ee/app/models/ee/merge_request.rb index 87f360969722b416bf2dfdae954e8515c77e2e34..f3ae5fc5495d5a2aa6b6bac880f874c87b440828 100644 --- a/ee/app/models/ee/merge_request.rb +++ b/ee/app/models/ee/merge_request.rb @@ -450,11 +450,10 @@ def synchronize_approval_rules_from_target_project return if merged? project_rules = target_project.approval_rules.report_approver.includes(:users, :groups) - feature_enabled = ::Feature.enabled?(:policy_mergability_check, project) project_rules.find_each do |project_rule| project_rule.apply_report_approver_rules_to(self) do |rule_attributes| - rule_attributes[:approvals_required] = 0 if feature_enabled && project_rule.from_scan_result_policy? + rule_attributes[:approvals_required] = 0 if project_rule.from_scan_result_policy? end end end diff --git a/ee/app/services/merge_requests/mergeability/check_security_policy_violations_service.rb b/ee/app/services/merge_requests/mergeability/check_security_policy_violations_service.rb index fd55abf2044f7d9c67f32251c06b89d75e364c36..97cc3fcc38c7d377272dbf43a775e26b589c2227 100644 --- a/ee/app/services/merge_requests/mergeability/check_security_policy_violations_service.rb +++ b/ee/app/services/merge_requests/mergeability/check_security_policy_violations_service.rb @@ -7,9 +7,7 @@ class CheckSecurityPolicyViolationsService < CheckBaseService description 'Checks whether the security policies are satisfied' def execute - if ::Feature.disabled?(:policy_mergability_check, - merge_request.project) || - !merge_request.project.licensed_feature_available?(:security_orchestration_policies) || + if !merge_request.project.licensed_feature_available?(:security_orchestration_policies) || merge_request.scan_result_policy_reads_through_approval_rules.none? return inactive end diff --git a/ee/app/services/security/security_orchestration_policies/update_violations_service.rb b/ee/app/services/security/security_orchestration_policies/update_violations_service.rb index a9d4123e44f20a9d3293a3bbb16a007205d3d233..6958335b2e9d417f19a5ffbc52a563b6167279ab 100644 --- a/ee/app/services/security/security_orchestration_policies/update_violations_service.rb +++ b/ee/app/services/security/security_orchestration_policies/update_violations_service.rb @@ -113,8 +113,6 @@ def violation_status_for_policy(policy, default_status) end def publish_violations_updated_event - return unless ::Feature.enabled?(:policy_mergability_check, merge_request.project) - ::Gitlab::EventStore.publish( ::MergeRequests::ViolationsUpdatedEvent.new( data: { merge_request_id: merge_request.id } diff --git a/ee/app/workers/security/scan_result_policies/unblock_pending_merge_request_violations_worker.rb b/ee/app/workers/security/scan_result_policies/unblock_pending_merge_request_violations_worker.rb index 2d7dce1831b83eacf64c1ef68412b18582eb3d9e..a5bbb6067a11de6b407ba57e216c674b21568ffc 100644 --- a/ee/app/workers/security/scan_result_policies/unblock_pending_merge_request_violations_worker.rb +++ b/ee/app/workers/security/scan_result_policies/unblock_pending_merge_request_violations_worker.rb @@ -14,7 +14,6 @@ class UnblockPendingMergeRequestViolationsWorker def perform(pipeline_id) pipeline = ::Ci::Pipeline.find_by_id(pipeline_id) || return project = pipeline.project - return if ::Feature.disabled?(:policy_mergability_check, project) return unless project.licensed_feature_available?(:security_orchestration_policies) pipeline.opened_merge_requests_with_head_sha.each do |merge_request| diff --git a/ee/app/workers/security/unenforceable_policy_rules_pipeline_notification_worker.rb b/ee/app/workers/security/unenforceable_policy_rules_pipeline_notification_worker.rb index 4aa3088ad8a4a25f8e5458f735721b12dced85a7..d56ab3aa9e4dad0510afe3470d6ee9c8beb58c9d 100644 --- a/ee/app/workers/security/unenforceable_policy_rules_pipeline_notification_worker.rb +++ b/ee/app/workers/security/unenforceable_policy_rules_pipeline_notification_worker.rb @@ -29,10 +29,8 @@ def perform(pipeline_id) related_merge_requests = pipeline.opened_merge_requests_with_head_sha return if related_merge_requests.none? - if ::Feature.enabled?(:policy_mergability_check, project) - Security::ScanResultPolicies::UnblockPendingMergeRequestViolationsWorker - .perform_in(UNBLOCK_PENDING_VIOLATIONS_TIMEOUT, pipeline_id) - end + Security::ScanResultPolicies::UnblockPendingMergeRequestViolationsWorker + .perform_in(UNBLOCK_PENDING_VIOLATIONS_TIMEOUT, pipeline_id) related_merge_requests.each do |merge_request| Security::UnenforceablePolicyRulesNotificationService.new(merge_request).execute diff --git a/ee/config/feature_flags/beta/policy_mergability_check.yml b/ee/config/feature_flags/beta/policy_mergability_check.yml deleted file mode 100644 index 06c30cfe715381e07f28b24db7942e224c05fd62..0000000000000000000000000000000000000000 --- a/ee/config/feature_flags/beta/policy_mergability_check.yml +++ /dev/null @@ -1,9 +0,0 @@ ---- -name: policy_mergability_check -feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/444459 -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/159077 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/473704 -milestone: '17.3' -group: group::code review -type: beta -default_enabled: true diff --git a/ee/spec/graphql/ee/types/merge_request_type_spec.rb b/ee/spec/graphql/ee/types/merge_request_type_spec.rb index 06161f59ad0409bb66758fabd614451c2f7d280c..6a021f186aa1590a7cbccee325d5fa8bfe929b50 100644 --- a/ee/spec/graphql/ee/types/merge_request_type_spec.rb +++ b/ee/spec/graphql/ee/types/merge_request_type_spec.rb @@ -363,8 +363,6 @@ def setup_approval_rules(merge_request) multiple_approval_rules: true, blocking_merge_requests: true ) - - stub_feature_flags(policy_mergability_check: true) end it_behaves_like 'avoids N+1 queries related to approval rules' diff --git a/ee/spec/lib/ee/gitlab/data_builder/pipeline_spec.rb b/ee/spec/lib/ee/gitlab/data_builder/pipeline_spec.rb index b6d5d32d950f990f6a6ddbfb9694b4c384d23776..ece0d0c205fd3bcabf1e5364fd3956c95f830a6c 100644 --- a/ee/spec/lib/ee/gitlab/data_builder/pipeline_spec.rb +++ b/ee/spec/lib/ee/gitlab/data_builder/pipeline_spec.rb @@ -196,8 +196,6 @@ def setup_approvals(merge_request, user, group_user) merge_request_approvers: true, multiple_approval_rules: true ) - - stub_feature_flags(policy_mergability_check: true) end it_behaves_like 'avoids N+1 queries related to approval rules' diff --git a/ee/spec/models/merge_request_spec.rb b/ee/spec/models/merge_request_spec.rb index d9bd7e4c630f248ec9314ed9b64ae04d421f5099..7df1eace808547202cbee5e8b5b201475e82441a 100644 --- a/ee/spec/models/merge_request_spec.rb +++ b/ee/spec/models/merge_request_spec.rb @@ -2059,19 +2059,6 @@ def create_mr(metrics_data = {}) expect(merge_request.approval_rules.map { |rule| [rule.report_type, rule.approvals_required] }) .to match_array([['license_scanning', 0], ['code_coverage', 2], ['scan_finding', 0], ['any_merge_request', 0]]) end - - context 'with feature flag policy_mergability_check false' do - before do - stub_feature_flags(policy_mergability_check: false) - end - - it 'does not reset any approval rules' do - subject - - expect(merge_request.approval_rules.map { |rule| [rule.report_type, rule.approvals_required] }) - .to match_array([['license_scanning', 2], ['code_coverage', 2], ['scan_finding', 2], ['any_merge_request', 2]]) - end - end end describe '#sync_project_approval_rules_for_policy_configuration' do diff --git a/ee/spec/services/merge_requests/mergeability/check_security_policy_violations_service_spec.rb b/ee/spec/services/merge_requests/mergeability/check_security_policy_violations_service_spec.rb index d101bbf840c1665aefb98732e56586ddeba3fcc4..881c9709e9a8f0a1d5e7db1453a57dbaae3c6ff0 100644 --- a/ee/spec/services/merge_requests/mergeability/check_security_policy_violations_service_spec.rb +++ b/ee/spec/services/merge_requests/mergeability/check_security_policy_violations_service_spec.rb @@ -34,16 +34,6 @@ scan_result_policy_read: policy, name: 'Policy 1') end - context 'when policy_mergability_check is false' do - before do - stub_feature_flags(policy_mergability_check: false) - end - - it 'returns a check result with inactive status' do - expect(result.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::INACTIVE_STATUS - end - end - context 'when security_orchestration_policies license is false' do before do stub_licensed_features(security_orchestration_policies: false) diff --git a/ee/spec/services/merge_requests/sync_report_approver_approval_rules_spec.rb b/ee/spec/services/merge_requests/sync_report_approver_approval_rules_spec.rb index 3f9a834b116c021ec59081e885b5406a684c3d38..a5f15d938f8c4d1ce96d983b2c75d672e1e18e1b 100644 --- a/ee/spec/services/merge_requests/sync_report_approver_approval_rules_spec.rb +++ b/ee/spec/services/merge_requests/sync_report_approver_approval_rules_spec.rb @@ -25,42 +25,6 @@ let!(:report_approval_project_rule) { create(:approval_project_rule, report_type, project: merge_request.target_project, approvals_required: 2, name: default_name) } let!(:regular_approval_project_rule) { create(:approval_project_rule, project: merge_request.target_project, approvals_required: 2) } - context 'when the policy_mergability_check feature flag is false' do - before do - stub_feature_flags(policy_mergability_check: false) - end - - context 'when report_approver_rules are enabled' do - it 'creates rule for report approvers' do - expect { service.execute } - .to change { merge_request.approval_rules.where(name: default_name).count }.from(0).to(1) - - rule = merge_request.approval_rules.find_by(name: default_name) - - expect(rule).to be_report_approver - expect(rule.report_type).to eq(report_type.to_s) - expect(rule.name).to eq(report_approval_project_rule.name) - expect(rule.approvals_required).to eq(report_approval_project_rule.approvals_required) - expect(rule.approval_project_rule).to eq(report_approval_project_rule) - end - - it 'updates previous report approval rule if defined' do - previous_rule = create(:report_approver_rule, report_type, merge_request: merge_request, approvals_required: 3) - create(:approval_merge_request_rule_source, approval_merge_request_rule: previous_rule, - approval_project_rule: report_approval_project_rule) - - expect { service.execute } - .not_to change { merge_request.approval_rules.count } - - expect(previous_rule.reload).to be_report_approver - expect(previous_rule.report_type).to eq(report_type.to_s) - expect(previous_rule.name).to eq(report_approval_project_rule.name) - expect(previous_rule.approvals_required).to eq(report_approval_project_rule.approvals_required) - expect(previous_rule.approval_project_rule).to eq(report_approval_project_rule) - end - end - end - context 'when report_approver_rules are enabled' do it 'creates rule for report approvers' do expect { service.execute } diff --git a/ee/spec/services/security/security_orchestration_policies/update_violations_service_spec.rb b/ee/spec/services/security/security_orchestration_policies/update_violations_service_spec.rb index 7736070fe7348c7c45e4b958e73087afee4950dd..3dc1e814834d8d64131b7fe3219d6eea8ff8320a 100644 --- a/ee/spec/services/security/security_orchestration_policies/update_violations_service_spec.rb +++ b/ee/spec/services/security/security_orchestration_policies/update_violations_service_spec.rb @@ -98,16 +98,6 @@ def last_violation .to publish_event(::MergeRequests::ViolationsUpdatedEvent) .with(merge_request_id: merge_request.id) end - - context 'when policy_mergability_check is off' do - before do - stub_feature_flags(policy_mergability_check: false) - end - - it 'does not publish MergeRequests::ViolationsUpdatedEvent' do - expect { service.execute }.not_to publish_event(MergeRequests::ViolationsUpdatedEvent) - end - end end context 'with pre-existing violations' do @@ -174,16 +164,6 @@ def last_violation .to publish_event(::MergeRequests::ViolationsUpdatedEvent) .with(merge_request_id: merge_request.id) end - - context 'when policy_mergability_check is off' do - before do - stub_feature_flags(policy_mergability_check: false) - end - - it 'does not publish MergeRequests::ViolationsUpdatedEvent' do - expect { service.execute }.not_to publish_event(MergeRequests::ViolationsUpdatedEvent) - end - end end context 'without violations' do diff --git a/ee/spec/workers/security/scan_result_policies/unblock_pending_merge_request_violations_worker_spec.rb b/ee/spec/workers/security/scan_result_policies/unblock_pending_merge_request_violations_worker_spec.rb index da24b4ec7261ab02294d216859c772a8285612f9..f68bf4fea7059a7197a3eee32e0bafbe29472350 100644 --- a/ee/spec/workers/security/scan_result_policies/unblock_pending_merge_request_violations_worker_spec.rb +++ b/ee/spec/workers/security/scan_result_policies/unblock_pending_merge_request_violations_worker_spec.rb @@ -135,15 +135,6 @@ end end - context 'when feature flag "policy_mergability_check" is disabled' do - before do - stub_feature_flags(policy_mergability_check: false) - end - - it_behaves_like 'does not update merge request violations' - it_behaves_like 'does not update merge request report_approver approvals' - end - context 'when feature is not licensed' do let(:feature_licensed) { false } diff --git a/ee/spec/workers/security/unenforceable_policy_rules_pipeline_notification_worker_spec.rb b/ee/spec/workers/security/unenforceable_policy_rules_pipeline_notification_worker_spec.rb index 8804354a7193c495505fa2e9596c65c1c42e1aa0..58db3ef2dbe24b252f97e978d376cb96745f6d5f 100644 --- a/ee/spec/workers/security/unenforceable_policy_rules_pipeline_notification_worker_spec.rb +++ b/ee/spec/workers/security/unenforceable_policy_rules_pipeline_notification_worker_spec.rb @@ -68,14 +68,6 @@ it_behaves_like 'does not schedule UnblockPendingMergeRequestViolationsWorker' end - - context 'when feature flag "policy_mergability_check" is disabled' do - before do - stub_feature_flags(policy_mergability_check: false) - end - - it_behaves_like 'does not schedule UnblockPendingMergeRequestViolationsWorker' - end end context 'when pipeline is manual' do