diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 23c2164135dc59b2b2e099a0f69ae7f410229108..10ebda4489b36ccf10036131fe3cc72d3adbe581 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -1132,6 +1132,14 @@ def complete_and_has_reports?(reports_scope) end end + def complete_or_manual_and_has_reports?(reports_scope) + return complete_and_has_reports?(reports_scope) unless include_manual_to_pipeline_completion_enabled? + + return latest_report_builds(reports_scope).exists? if Feature.enabled?(:mr_show_reports_immediately, project, type: :development) + + complete_or_manual? && has_reports?(reports_scope) + end + def has_coverage_reports? pipeline_artifacts&.report_exists?(:code_coverage) end @@ -1401,6 +1409,12 @@ def auto_cancel_on_new_commit pipeline_metadata&.auto_cancel_on_new_commit || 'conservative' end + def include_manual_to_pipeline_completion_enabled? + strong_memoize(:include_manual_to_pipeline_completion_enabled) do + ::Feature.enabled?(:include_manual_to_pipeline_completion, self.project, type: :gitlab_com_derisk) + end + end + private def add_message(severity, content) diff --git a/app/models/concerns/ci/has_status.rb b/app/models/concerns/ci/has_status.rb index fb2b12e5f0045e1a57cbe5e9bc931aae9e634168..cf7442d186362cfc2e420a5b992a6d3585d385bd 100644 --- a/app/models/concerns/ci/has_status.rb +++ b/app/models/concerns/ci/has_status.rb @@ -47,6 +47,10 @@ def completed_statuses COMPLETED_STATUSES.map(&:to_sym) end + def completed_with_manual_statuses + completed_statuses + [:manual] + end + def stopped_statuses STOPPED_STATUSES.map(&:to_sym) end @@ -114,6 +118,10 @@ def complete? COMPLETED_STATUSES.include?(status) end + def complete_or_manual? + self.class.completed_with_manual_statuses.map(&:to_s).include?(status) + end + def incomplete? COMPLETED_STATUSES.exclude?(status) end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 3f9fc44bd51654e4500a41215c444b5e675ac86d..b637ba87743922ba37e59988dba68c6bc29367a4 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1840,11 +1840,11 @@ def compare_reports(service_class, current_user = nil, report_type = nil, additi end def has_sast_reports? - !!actual_head_pipeline&.complete_and_has_reports?(::Ci::JobArtifact.of_report_type(:sast)) + !!actual_head_pipeline&.complete_or_manual_and_has_reports?(::Ci::JobArtifact.of_report_type(:sast)) end def has_secret_detection_reports? - !!actual_head_pipeline&.complete_and_has_reports?(::Ci::JobArtifact.of_report_type(:secret_detection)) + !!actual_head_pipeline&.complete_or_manual_and_has_reports?(::Ci::JobArtifact.of_report_type(:secret_detection)) end def compare_sast_reports(current_user) diff --git a/config/feature_flags/gitlab_com_derisk/include_manual_to_pipeline_completion.yml b/config/feature_flags/gitlab_com_derisk/include_manual_to_pipeline_completion.yml new file mode 100644 index 0000000000000000000000000000000000000000..e68c11f5490c2384d2450e68aec5f2fc866ee36e --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/include_manual_to_pipeline_completion.yml @@ -0,0 +1,9 @@ +--- +name: include_manual_to_pipeline_completion +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/439691 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/143046 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/440652 +milestone: '16.10' +group: group::threat insights +type: gitlab_com_derisk +default_enabled: false diff --git a/ee/app/models/ee/ci/pipeline.rb b/ee/app/models/ee/ci/pipeline.rb index 498ffc77ee39c49cc66b60f92f19619a423628a5..c42275eb54536928b835539bd2ebf6504ec95e23 100644 --- a/ee/app/models/ee/ci/pipeline.rb +++ b/ee/app/models/ee/ci/pipeline.rb @@ -64,7 +64,8 @@ module Pipeline }.freeze state_machine :status do - after_transition any => ::Ci::Pipeline.completed_statuses do |pipeline| + after_transition any => ::Ci::Pipeline.completed_with_manual_statuses do |pipeline| + next if pipeline.manual? && !pipeline.include_manual_to_pipeline_completion_enabled? next unless pipeline.can_store_security_reports? pipeline.run_after_commit do @@ -80,7 +81,8 @@ module Pipeline end end - after_transition any => ::Ci::Pipeline.completed_statuses do |pipeline| + after_transition any => ::Ci::Pipeline.completed_with_manual_statuses do |pipeline| + next if pipeline.manual? && !pipeline.include_manual_to_pipeline_completion_enabled? next if pipeline.can_store_security_reports? next if pipeline.child? next unless pipeline.default_branch? && pipeline.can_ingest_sbom_reports? @@ -90,7 +92,9 @@ module Pipeline end end - after_transition any => ::Ci::Pipeline.completed_statuses do |pipeline| + after_transition any => ::Ci::Pipeline.completed_with_manual_statuses do |pipeline| + next if pipeline.manual? && !pipeline.include_manual_to_pipeline_completion_enabled? + pipeline.run_after_commit do ::Ci::SyncReportsToReportApprovalRulesWorker.perform_async(pipeline.id) end @@ -201,7 +205,7 @@ def can_ingest_sbom_reports? end def has_sbom_reports? - complete_and_has_reports?(::Ci::JobArtifact.of_report_type(:sbom)) + complete_or_manual_and_has_reports?(::Ci::JobArtifact.of_report_type(:sbom)) end def can_store_security_reports? @@ -255,7 +259,9 @@ def self_and_descendant_security_scans end def has_security_reports? - complete_and_has_reports?(::Ci::JobArtifact.security_reports.or(::Ci::JobArtifact.of_report_type(:license_scanning))) + security_and_license_scanning_file_types = Ci::JobArtifact::SECURITY_REPORT_FILE_TYPES | %w[license_scanning] + + complete_or_manual_and_has_reports?(::Ci::JobArtifact.with_file_types(security_and_license_scanning_file_types)) end def has_repository_xray_reports? diff --git a/ee/app/models/ee/merge_request.rb b/ee/app/models/ee/merge_request.rb index 2abfa3d059fd29ad0c409da9862539d187c9ad0a..3c96eda175db2e4535b675af0abf9da76d3e9026 100644 --- a/ee/app/models/ee/merge_request.rb +++ b/ee/app/models/ee/merge_request.rb @@ -288,11 +288,11 @@ def enabled_reports end def has_security_reports? - !!actual_head_pipeline&.complete_and_has_reports?(::Ci::JobArtifact.security_reports) + !!actual_head_pipeline&.complete_or_manual_and_has_reports?(::Ci::JobArtifact.security_reports) end def has_dependency_scanning_reports? - !!actual_head_pipeline&.complete_and_has_reports?(::Ci::JobArtifact.of_report_type(:dependency_list)) + !!actual_head_pipeline&.complete_or_manual_and_has_reports?(::Ci::JobArtifact.of_report_type(:dependency_list)) end def compare_dependency_scanning_reports(current_user) @@ -302,7 +302,7 @@ def compare_dependency_scanning_reports(current_user) end def has_container_scanning_reports? - !!actual_head_pipeline&.complete_and_has_reports?(::Ci::JobArtifact.of_report_type(:container_scanning)) + !!actual_head_pipeline&.complete_or_manual_and_has_reports?(::Ci::JobArtifact.of_report_type(:container_scanning)) end def compare_container_scanning_reports(current_user) @@ -312,7 +312,7 @@ def compare_container_scanning_reports(current_user) end def has_dast_reports? - !!actual_head_pipeline&.complete_and_has_reports?(::Ci::JobArtifact.of_report_type(:dast)) + !!actual_head_pipeline&.complete_or_manual_and_has_reports?(::Ci::JobArtifact.of_report_type(:dast)) end def compare_dast_reports(current_user) @@ -353,7 +353,7 @@ def compare_metrics_reports end def has_coverage_fuzzing_reports? - !!actual_head_pipeline&.complete_and_has_reports?(::Ci::JobArtifact.of_report_type(:coverage_fuzzing)) + !!actual_head_pipeline&.complete_or_manual_and_has_reports?(::Ci::JobArtifact.of_report_type(:coverage_fuzzing)) end def compare_coverage_fuzzing_reports(current_user) @@ -363,7 +363,7 @@ def compare_coverage_fuzzing_reports(current_user) end def has_api_fuzzing_reports? - !!actual_head_pipeline&.complete_and_has_reports?(::Ci::JobArtifact.of_report_type(:api_fuzzing)) + !!actual_head_pipeline&.complete_or_manual_and_has_reports?(::Ci::JobArtifact.of_report_type(:api_fuzzing)) end def compare_api_fuzzing_reports(current_user) diff --git a/ee/app/services/security/store_scans_service.rb b/ee/app/services/security/store_scans_service.rb index c855b79f935f5aa79632bc2e589d00520619b6b0..1435dceb043d3ca8c0756e8adbc0dabf100ca65d 100644 --- a/ee/app/services/security/store_scans_service.rb +++ b/ee/app/services/security/store_scans_service.rb @@ -13,6 +13,20 @@ def initialize(pipeline) def execute return if already_purged? + return old_execute unless pipeline.include_manual_to_pipeline_completion_enabled? + + # StoreGroupedScansService returns true only when it creates a `security_scans` record. + # To avoid resource wastage we are skipping the reports ingestion and rules sync when there are no new scans. + results = grouped_report_artifacts.map { |artifacts| StoreGroupedScansService.execute(artifacts) } + + return unless results.any?(true) + + schedule_store_reports_worker + schedule_scan_security_report_secrets_worker + sync_findings_to_approval_rules unless pipeline.default_branch? + end + + def old_execute grouped_report_artifacts.each { |artifacts| StoreGroupedScansService.execute(artifacts) } schedule_store_reports_worker diff --git a/ee/lib/gitlab/license_scanning/sbom_scanner.rb b/ee/lib/gitlab/license_scanning/sbom_scanner.rb index 4d7b40c1648b5b36ba74e1e6f46b58042ea46c4a..bab6e7271955bb2a8449037eb00ab66f8a7fb2d3 100644 --- a/ee/lib/gitlab/license_scanning/sbom_scanner.rb +++ b/ee/lib/gitlab/license_scanning/sbom_scanner.rb @@ -45,7 +45,7 @@ def has_data? def results_available? return false if pipeline.blank? - pipeline.complete_and_has_reports?(::Ci::JobArtifact.of_report_type(:sbom)) + pipeline.complete_or_manual_and_has_reports?(::Ci::JobArtifact.of_report_type(:sbom)) end def latest_build_for_default_branch diff --git a/ee/spec/lib/gitlab/license_scanning/sbom_scanner_spec.rb b/ee/spec/lib/gitlab/license_scanning/sbom_scanner_spec.rb index 5d48fcc571be93e62f163930d6263dc77be7185f..bb3b53b98e02886a66332d523d4859789cb344a7 100644 --- a/ee/spec/lib/gitlab/license_scanning/sbom_scanner_spec.rb +++ b/ee/spec/lib/gitlab/license_scanning/sbom_scanner_spec.rb @@ -170,6 +170,14 @@ it { is_expected.to be_truthy } end end + + context "and the pipeline is blocked by manual jobs" do + before do + pipeline.block! + end + + it { is_expected.to be_truthy } + end end context "when the pipeline does not have an sbom report" do diff --git a/ee/spec/models/ci/pipeline_spec.rb b/ee/spec/models/ci/pipeline_spec.rb index 3ae6ad9bb69b70d0edb906599c9cda54810e2d81..987f00d50b23238d3f836878ad63116b28017b34 100644 --- a/ee/spec/models/ci/pipeline_spec.rb +++ b/ee/spec/models/ci/pipeline_spec.rb @@ -204,20 +204,46 @@ end end - context 'when pipeline is succeeded' do - it_behaves_like 'storing the security scans', :succeed - end + shared_examples_for 'completed statuses scans are stored' do + context 'when pipeline is succeeded' do + it_behaves_like 'storing the security scans', :succeed + end - context 'when pipeline is dropped' do - it_behaves_like 'storing the security scans', :drop + context 'when pipeline is dropped' do + it_behaves_like 'storing the security scans', :drop + end + + context 'when pipeline is skipped' do + it_behaves_like 'storing the security scans', :skip + end + + context 'when pipeline is canceled' do + it_behaves_like 'storing the security scans', :cancel + end end - context 'when pipeline is skipped' do - it_behaves_like 'storing the security scans', :skip + it_behaves_like 'completed statuses scans are stored' + + context 'when pipeline is blocked' do + it_behaves_like 'storing the security scans', :block end - context 'when pipeline is canceled' do - it_behaves_like 'storing the security scans', :cancel + context 'when include_manual_to_pipeline_completion is false' do + before do + stub_feature_flags(include_manual_to_pipeline_completion: false) + end + + it_behaves_like 'completed statuses scans are stored' + + context 'when pipeline is blocked' do + it 'does not schedule store security scans job' do + allow(::Security::StoreScansWorker).to receive(:perform_async) + + pipeline.update!(status_event: :block) + + expect(::Security::StoreScansWorker).not_to have_received(:perform_async) + end + end end end @@ -379,22 +405,51 @@ end end - context 'when transitioning to a completed status' do - where(:transition) { %i[succeed drop skip cancel] } + context 'when transitioning to completed or blocked status' do + where(:transition) { %i[succeed drop skip cancel block] } with_them do it_behaves_like 'ingesting sbom reports' end + + context 'with include_manual_to_pipeline_completion FF disabled' do + before do + stub_feature_flags(include_manual_to_pipeline_completion: false) + end + + context 'for completed status' do + where(:transition) { %i[succeed drop skip cancel] } + + with_them do + it_behaves_like 'ingesting sbom reports' + end + end + + context 'for manual status' do + where(:transition) do + %i[ + block + ] + end + + with_them do + it 'does not ingest sbom reports' do + transition_pipeline + + expect(::Sbom::IngestReportsWorker).not_to have_received(:perform_async) + end + end + end + end end - context 'when transitioning to a non-completed status' do + context 'when transitioning to a non-completed status except block' do where(:transition) do %i[ enqueue request_resource prepare run - block delay ] end @@ -406,6 +461,31 @@ expect(::Sbom::IngestReportsWorker).not_to have_received(:perform_async) end end + + context 'with include_manual_to_pipeline_completion FF disabled' do + before do + stub_feature_flags(include_manual_to_pipeline_completion: false) + end + + where(:transition) do + %i[ + enqueue + request_resource + prepare + block + run + delay + ] + end + + with_them do + it 'does not ingest sbom reports' do + transition_pipeline + + expect(::Sbom::IngestReportsWorker).not_to have_received(:perform_async) + end + end + end end end @@ -521,17 +601,59 @@ end describe 'state machine transitions' do - context 'on pipeline complete' do + context 'Ci::SyncReportsToReportApprovalRulesWorker' do let(:pipeline) { create(:ci_empty_pipeline, status: from_status) } - Ci::HasStatus::ACTIVE_STATUSES.each do |status| - context "from #{status}" do - let(:from_status) { status } + shared_examples 'schedules worker' do + Ci::HasStatus::ACTIVE_STATUSES.each do |status| + context "from #{status}" do + let(:from_status) { status } - it 'schedules Ci::SyncReportsToReportApprovalRulesWorker' do - expect(Ci::SyncReportsToReportApprovalRulesWorker).to receive(:perform_async).with(pipeline.id) + it do + expect(Ci::SyncReportsToReportApprovalRulesWorker).to receive(:perform_async).with(pipeline.id) - pipeline.succeed + transition_pipeline + end + end + end + end + + context 'on pipeline complete' do + subject(:transition_pipeline) { pipeline.succeed } + + it_behaves_like 'schedules worker' + end + + context 'on pipeline manual' do + subject(:transition_pipeline) { pipeline.block } + + it_behaves_like 'schedules worker' + end + + context 'when include_manual_to_pipeline_completion is disabled' do + before do + stub_feature_flags(include_manual_to_pipeline_completion: false) + end + + context 'on pipeline complete' do + subject(:transition_pipeline) { pipeline.succeed } + + it_behaves_like 'schedules worker' + end + + context 'on pipeline manual' do + subject(:transition_pipeline) { pipeline.block } + + Ci::HasStatus::ACTIVE_STATUSES.each do |status| + context "from #{status}" do + let(:from_status) { status } + + it 'does not schedule worker' do + expect(Ci::SyncReportsToReportApprovalRulesWorker).not_to receive(:perform_async) + + transition_pipeline + end + end end end end @@ -808,6 +930,14 @@ end it { is_expected.to be_truthy } + + context 'when the pipeline is blocked by manual jobs' do + before do + pipeline.block! + end + + it { is_expected.to be_truthy } + end end end end diff --git a/ee/spec/models/merge_request_spec.rb b/ee/spec/models/merge_request_spec.rb index 52004b7ee7c8d1f3e9de24f229ef12155459c0fd..aaced0ab71a58ee0f0703577052300a627b31f3e 100644 --- a/ee/spec/models/merge_request_spec.rb +++ b/ee/spec/models/merge_request_spec.rb @@ -684,6 +684,14 @@ let(:merge_request) { create(:ee_merge_request, :with_dast_reports, source_project: project) } it { is_expected.to be_truthy } + + context 'when head pipeline is blocked by manual jobs' do + before do + merge_request.actual_head_pipeline.block! + end + + it { is_expected.to be_truthy } + end end context 'when head pipeline does not have security reports' do @@ -704,6 +712,14 @@ let(:merge_request) { create(:ee_merge_request, :with_dependency_scanning_reports, source_project: project) } it { is_expected.to be_truthy } + + context 'when head pipeline is blocked by manual jobs' do + before do + merge_request.actual_head_pipeline.block! + end + + it { is_expected.to be_truthy } + end end context 'when head pipeline does not have dependency scanning reports' do @@ -724,6 +740,14 @@ let(:merge_request) { create(:ee_merge_request, :with_container_scanning_reports, source_project: project) } it { is_expected.to be_truthy } + + context 'when head pipeline is blocked by manual jobs' do + before do + merge_request.actual_head_pipeline.block! + end + + it { is_expected.to be_truthy } + end end context 'when head pipeline does not have container scanning reports' do @@ -744,6 +768,14 @@ let(:merge_request) { create(:ee_merge_request, :with_dast_reports, source_project: project) } it { is_expected.to be_truthy } + + context 'when head pipeline is blocked by manual jobs' do + before do + merge_request.actual_head_pipeline.block! + end + + it { is_expected.to be_truthy } + end end context 'when pipeline ran for an older commit than the branch head' do @@ -791,6 +823,14 @@ let(:merge_request) { create(:ee_merge_request, :with_coverage_fuzzing_reports, source_project: project) } it { is_expected.to be_truthy } + + context 'when head pipeline is blocked by manual jobs' do + before do + merge_request.actual_head_pipeline.block! + end + + it { is_expected.to be_truthy } + end end context 'when head pipeline does not have coverage fuzzing reports' do @@ -811,6 +851,14 @@ let(:merge_request) { create(:ee_merge_request, :with_api_fuzzing_reports, source_project: project) } it { is_expected.to be_truthy } + + context 'when head pipeline is blocked by manual jobs' do + before do + merge_request.actual_head_pipeline.block! + end + + it { is_expected.to be_truthy } + end end context 'when head pipeline does not have coverage fuzzing reports' do @@ -2468,9 +2516,9 @@ def stub_foss_conditions_met context 'when no base pipeline has completed' do before do - base_pipeline.update!(status: :manual) - old_base_pipeline.update!(status: :manual) - most_recent_base_pipeline.update!(status: :manual) + base_pipeline.update!(status: :waiting_for_resource) + old_base_pipeline.update!(status: :waiting_for_resource) + most_recent_base_pipeline.update!(status: :waiting_for_resource) end context 'when comparison prior to pipeline completion is disabled' do diff --git a/ee/spec/services/security/store_scans_service_spec.rb b/ee/spec/services/security/store_scans_service_spec.rb index 8c3d0ca431d353fc6185c21b2c6a8aac6b22678e..3477829f4e05a40cc885a6cd4c23855f9826b595 100644 --- a/ee/spec/services/security/store_scans_service_spec.rb +++ b/ee/spec/services/security/store_scans_service_spec.rb @@ -65,142 +65,201 @@ end context 'when the pipeline does not have a purged security scan' do - it 'executes Security::StoreGroupedScansService for each group of artifacts if the feature is available' do - store_group_of_artifacts - - expect(Security::StoreGroupedScansService).to have_received(:execute).with([sast_artifact]) - expect(Security::StoreGroupedScansService).not_to have_received(:execute).with([dast_artifact]) - end + shared_examples 'executes service and workers' do + context 'for Security::StoreGroupedScansService' do + it 'executes only for artifacts where the feature is available' do + store_group_of_artifacts - context 'when the pipeline is for the default branch' do - before do - allow(pipeline).to receive(:default_branch?).and_return(true) + expect(Security::StoreGroupedScansService).to have_received(:execute).with([sast_artifact]) + expect(Security::StoreGroupedScansService).not_to have_received(:execute).with([dast_artifact]) + end end - it 'schedules the `StoreSecurityReportsWorker`' do - store_group_of_artifacts - - expect(StoreSecurityReportsWorker).to have_received(:perform_async).with(pipeline.id) - end - end + context 'for StoreSecurityReportsWorker' do + context 'when the pipeline is for the default branch' do + before do + allow(pipeline).to receive(:default_branch?).and_return(true) + end - context 'when the pipeline is not for the default branch' do - before do - allow(pipeline).to receive(:default_branch?).and_return(false) - end + it 'schedules the `StoreSecurityReportsWorker`' do + store_group_of_artifacts - it 'does not schedule the `StoreSecurityReportsWorker`' do - store_group_of_artifacts + expect(StoreSecurityReportsWorker).to have_received(:perform_async).with(pipeline.id) + end + end - expect(StoreSecurityReportsWorker).not_to have_received(:perform_async) - end - end + context 'when the pipeline is not for the default branch' do + before do + allow(pipeline).to receive(:default_branch?).and_return(false) + end - shared_examples 'does not revoke secret detection tokens' do - it 'does not schedule the `ScanSecurityReportSecretsWorker`' do - store_group_of_artifacts + it 'does not schedule the `StoreSecurityReportsWorker`' do + store_group_of_artifacts - expect(ScanSecurityReportSecretsWorker).not_to have_received(:perform_async) + expect(StoreSecurityReportsWorker).not_to have_received(:perform_async) + end + end end - end - describe 'scheduling the `ScanSecurityReportSecretsWorker `' do - context 'when no secret detection security scans exist for the pipeline' do - before do - pipeline.project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + context 'for ScanSecurityReportSecretsWorker' do + shared_examples 'does not revoke secret detection tokens' do + it 'does not schedule the `ScanSecurityReportSecretsWorker`' do + store_group_of_artifacts - allow(Gitlab::CurrentSettings).to receive(:secret_detection_token_revocation_enabled?).and_return(true) + expect(ScanSecurityReportSecretsWorker).not_to have_received(:perform_async) + end end - include_examples 'does not revoke secret detection tokens' - end + describe 'scheduling the `ScanSecurityReportSecretsWorker `' do + context 'when no secret detection security scans exist for the pipeline' do + before do + pipeline.project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) - context 'when secret detection security scans exist for the pipeline' do - let_it_be(:scan) { create(:security_scan, scan_type: :secret_detection, build: sast_build) } - let_it_be(:finding) { create(:security_finding, :with_finding_data, scan: scan) } + allow(Gitlab::CurrentSettings).to receive(:secret_detection_token_revocation_enabled?).and_return(true) + end - context 'and the pipeline is in a private project' do - before do - pipeline.project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + include_examples 'does not revoke secret detection tokens' + end + + context 'when secret detection security scans exist for the pipeline' do + let_it_be(:scan) { create(:security_scan, scan_type: :secret_detection, build: sast_build) } + let_it_be(:finding) { create(:security_finding, :with_finding_data, scan: scan) } + + context 'and the pipeline is in a private project' do + before do + pipeline.project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + + allow(Gitlab::CurrentSettings).to receive( + :secret_detection_token_revocation_enabled?).and_return(false) + end + + include_examples 'does not revoke secret detection tokens' + end + + context 'and secret detection token revocation setting is disabled' do + before do + pipeline.project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + + allow(Gitlab::CurrentSettings).to receive( + :secret_detection_token_revocation_enabled?).and_return(false) + end - allow(Gitlab::CurrentSettings).to receive(:secret_detection_token_revocation_enabled?).and_return(false) + include_examples 'does not revoke secret detection tokens' + end + + context 'and the pipeline is in a public project and the setting is enabled' do + before do + pipeline.project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + + allow(Gitlab::CurrentSettings).to receive( + :secret_detection_token_revocation_enabled?).and_return(true) + end + + it 'schedules the `ScanSecurityReportSecretsWorker`' do + store_group_of_artifacts + + expect(ScanSecurityReportSecretsWorker).to have_received(:perform_async).with(pipeline.id) + end + end end + end + end - include_examples 'does not revoke secret detection tokens' + context 'for SyncFindingsToApprovalRulesWorker with scan result policies' do + let(:security_orchestration_policy_configuration) do + create(:security_orchestration_policy_configuration, project: pipeline.project) end - context 'and secret detection token revocation setting is disabled' do - before do - pipeline.project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + before do + allow(pipeline.project).to receive(:all_security_orchestration_policy_configurations) + .and_return([security_orchestration_policy_configuration]) + end - allow(Gitlab::CurrentSettings).to receive(:secret_detection_token_revocation_enabled?).and_return(false) + context 'when security_orchestration_policies is not licensed' do + before do + stub_licensed_features(security_orchestration_policies: false) end - include_examples 'does not revoke secret detection tokens' + it 'does not call SyncFindingsToApprovalRulesWorker' do + expect(Security::ScanResultPolicies::SyncFindingsToApprovalRulesWorker).not_to receive(:perform_async) + + store_group_of_artifacts + end end - context 'and the pipeline is in a public project and the setting is enabled' do + context 'when security_orchestration_policies is licensed' do before do - pipeline.project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + stub_licensed_features(security_orchestration_policies: true, sast: true) + end + + context 'when the pipeline is not for the default branch' do + before do + allow(pipeline).to receive(:default_branch?).and_return(false) + end - allow(Gitlab::CurrentSettings).to receive(:secret_detection_token_revocation_enabled?).and_return(true) + it 'calls SyncFindingsToApprovalRulesWorker' do + expect(Security::ScanResultPolicies::SyncFindingsToApprovalRulesWorker) + .to receive(:perform_async).with(pipeline.id) + + store_group_of_artifacts + end end - it 'schedules the `ScanSecurityReportSecretsWorker`' do - store_group_of_artifacts + context 'when the pipeline is for the default branch' do + before do + allow(pipeline).to receive(:default_branch?).and_return(true) + end + + it 'does not call SyncFindingsToApprovalRulesWorker' do + expect(Security::ScanResultPolicies::SyncFindingsToApprovalRulesWorker).not_to receive(:perform_async) - expect(ScanSecurityReportSecretsWorker).to have_received(:perform_async).with(pipeline.id) + store_group_of_artifacts + end end end end end - end - - context 'with scan result policies' do - let(:security_orchestration_policy_configuration) do - create(:security_orchestration_policy_configuration, project: pipeline.project) - end - before do - allow(pipeline.project).to receive(:all_security_orchestration_policy_configurations) - .and_return([security_orchestration_policy_configuration]) - end - - context 'when security_orchestration_policies is not licensed' do + context 'when StoreGroupedScansService.execute return false' do before do - stub_licensed_features(security_orchestration_policies: false) + allow(Security::StoreGroupedScansService).to receive(:execute).and_return(false) end - it 'does not call SyncFindingsToApprovalRulesWorker' do - expect(Security::ScanResultPolicies::SyncFindingsToApprovalRulesWorker).not_to receive(:perform_async) - + it 'does not schedule the `ScanSecurityReportSecretsWorker`' do store_group_of_artifacts + + expect(ScanSecurityReportSecretsWorker).not_to have_received(:perform_async) end - end - context 'when security_orchestration_policies is licensed' do - before do - stub_licensed_features(security_orchestration_policies: true) + it 'does not schedule the `StoreSecurityReportsWorker`' do + store_group_of_artifacts + + expect(StoreSecurityReportsWorker).not_to have_received(:perform_async) end - it 'calls SyncFindingsToApprovalRulesWorker' do - expect(Security::ScanResultPolicies::SyncFindingsToApprovalRulesWorker) - .to receive(:perform_async).with(pipeline.id) + it 'does not schedule `SyncFindingsToApprovalRulesWorker`' do + allow(Security::ScanResultPolicies::SyncFindingsToApprovalRulesWorker).to receive(:perform_async) store_group_of_artifacts + + expect(Security::ScanResultPolicies::SyncFindingsToApprovalRulesWorker).not_to have_received(:perform_async) end + end - context 'when the pipeline is for the default branch' do - before do - allow(pipeline).to receive(:default_branch?).and_return(true) - end + context 'when StoreGroupedScansService.execute return true' do + before do + allow(Security::StoreGroupedScansService).to receive(:execute).and_return(true) + end - it 'does not call SyncFindingsToApprovalRulesWorker' do - expect(Security::ScanResultPolicies::SyncFindingsToApprovalRulesWorker).not_to receive(:perform_async) + it_behaves_like 'executes service and workers' + end - store_group_of_artifacts - end + context 'when include_manual_to_pipeline_completion is disabled' do + before do + allow(pipeline).to receive(:include_manual_to_pipeline_completion_enabled?).and_return(false) end + + it_behaves_like 'executes service and workers' end end end diff --git a/spec/factories/ci/pipelines.rb b/spec/factories/ci/pipelines.rb index df4ffc4f0278972b36b5e989d77516ecaf30dd80..f5e9e63887f2559b35b0ec56091b22d43d01036f 100644 --- a/spec/factories/ci/pipelines.rb +++ b/spec/factories/ci/pipelines.rb @@ -82,6 +82,10 @@ status { :success } end + trait :manual do + status { :manual } + end + trait :running do started_at { Time.current } status { :running } diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index bac3ea5b0212a0653f32fa218f0ed67d897ced6e..5b1754d8946637c1c14ca565906a4db0b5394def 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -4380,6 +4380,97 @@ def create_pipeline(status, ref, sha) end end + describe '#complete_or_manual_and_has_reports?' do + subject(:complete_or_manual_and_has_reports?) do + pipeline.complete_or_manual_and_has_reports?( + Ci::JobArtifact.of_report_type(:test)) + end + + context 'when pipeline has builds' do + let(:pipeline) { create(:ci_pipeline) } + + before do + create(:ci_build, :test_reports, pipeline: pipeline) + end + + context 'with mr_show_reports_immediately flag enabled' do + before do + stub_feature_flags(mr_show_reports_immediately: project) + end + + it { expect(subject).to be_truthy } + end + + context 'with mr_show_reports_immediately flag disabled' do + before do + stub_feature_flags(mr_show_reports_immediately: false) + end + + it { expect(subject).to be_falsey } + + context 'when pipeline status is running' do + let(:pipeline) { create(:ci_pipeline, :running) } + + it { is_expected.to be_falsey } + end + + context 'when pipeline status is success' do + let(:pipeline) { create(:ci_pipeline, :success) } + + it { is_expected.to be_truthy } + end + + context 'when pipeline status is manual' do + let(:pipeline) { create(:ci_pipeline, :manual) } + + it { is_expected.to be_truthy } + end + end + end + + context 'when pipeline does not have builds' do + before do + create(:ci_build, :artifacts, pipeline: pipeline) + end + + context 'when pipeline status is success' do + let(:pipeline) { create(:ci_pipeline, :success) } + + it { is_expected.to be_falsey } + end + + context 'when pipeline status is manual' do + let(:pipeline) { create(:ci_pipeline, :manual) } + + it { is_expected.to be_falsey } + end + end + + context 'when pipeline has retried build' do + before do + create(:ci_build, :retried, :test_reports, pipeline: pipeline) + end + + context 'when pipeline status is running' do + let(:pipeline) { create(:ci_pipeline, :running) } + + it { is_expected.to be_falsey } + end + + context 'when pipeline status is success' do + let(:pipeline) { create(:ci_pipeline, :success) } + + it { is_expected.to be_falsey } + end + + context 'when pipeline status is manual' do + let(:pipeline) { create(:ci_pipeline, :manual) } + + it { is_expected.to be_falsey } + end + end + end + describe '#has_coverage_reports?' do subject { pipeline.has_coverage_reports? } diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index b72a23332d2db11ce9bc4dcb98912cf4bcfb750a..a035c594cd1327c044fd48643d993e840fef52cf 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -2576,6 +2576,14 @@ def set_compare(merge_request) let(:merge_request) { create(:merge_request, :with_sast_reports, source_project: project) } it { is_expected.to be_truthy } + + context 'when head pipeline is blocked by manual jobs' do + before do + merge_request.actual_head_pipeline.block! + end + + it { is_expected.to be_truthy } + end end context 'when head pipeline does not have sast reports' do @@ -2598,6 +2606,14 @@ def set_compare(merge_request) let(:merge_request) { create(:merge_request, :with_secret_detection_reports, source_project: project) } it { is_expected.to be_truthy } + + context 'when head pipeline is blocked by manual jobs' do + before do + merge_request.actual_head_pipeline.block! + end + + it { is_expected.to be_truthy } + end end context 'when head pipeline does not have secrets detection reports' do