Skip to content
Snippets Groups Projects
Verified Commit 5aabb662 authored by Zamir Martins's avatar Zamir Martins :speech_balloon: Committed by GitLab
Browse files

Revert "Merge branch 'allow_multiple_artifacts_per_job' into 'master'"

This reverts merge request !176101
parent c5f3ee35
No related branches found
No related tags found
3 merge requests!181325Fix ambiguous `created_at` in project.rb,!179611Draft: Rebase CR approach for zoekt assignments,!178327Revert "Merge branch 'allow_multiple_artifacts_per_job' into 'master'"
Pipeline #1630155201 passed
Pipeline: rspec:predictive

#1630160299

    Pipeline: rspec-ee:predictive

    #1630160289

      Pipeline: Ruby 3.2.6 as-if-foss

      #1630157022

        ......@@ -5,22 +5,21 @@ module Security
        class StoreFindingsService < ::BaseService
        BATCH_SIZE = 50
        attr_reader :security_scan, :report, :deduplicated_finding_uuids, :multiple_artifacts_allowed
        attr_reader :security_scan, :report, :deduplicated_finding_uuids
        def self.execute(security_scan, scanner, report, deduplicated_finding_uuids, multiple_artifacts_allowed)
        new(security_scan, scanner, report, deduplicated_finding_uuids, multiple_artifacts_allowed).execute
        def self.execute(security_scan, scanner, report, deduplicated_finding_uuids)
        new(security_scan, scanner, report, deduplicated_finding_uuids).execute
        end
        def initialize(security_scan, scanner, report, deduplicated_finding_uuids, multiple_artifacts_allowed)
        def initialize(security_scan, scanner, report, deduplicated_finding_uuids)
        @security_scan = security_scan
        @scanner = scanner
        @report = report
        @deduplicated_finding_uuids = deduplicated_finding_uuids
        @multiple_artifacts_allowed = multiple_artifacts_allowed
        end
        def execute
        return error('Findings are already stored!') if already_stored? && !multiple_artifacts_allowed
        return error('Findings are already stored!') if already_stored?
        store_findings
        success
        ......
        ......@@ -68,12 +68,10 @@ def initial_scan_status
        end
        def store_findings
        StoreFindingsService.execute(security_scan, vulnerability_scanner, security_report, register_finding_keys, multiple_artifacts_allowed?).then do |result|
        StoreFindingsService.execute(security_scan, vulnerability_scanner, security_report, register_finding_keys).then do |result|
        # If `StoreFindingsService` returns error, it means the findings
        # have already been stored before so we may re-run the deduplication logic.
        if result[:status] == :error && !multiple_artifacts_allowed? && deduplicate_findings?
        update_deduplicated_findings
        end
        update_deduplicated_findings if result[:status] == :error && deduplicate_findings?
        end
        security_scan.succeeded!
        ......@@ -131,11 +129,5 @@ def vulnerability_scanner
        scanner.assign_attributes(security_report.primary_scanner.to_hash)
        end
        end
        def multiple_artifacts_allowed?
        return false if ::Feature.disabled?(:dependency_scanning_for_pipelines_with_cyclonedx_reports, project)
        vulnerability_scanner.vulnerability_scanner?
        end
        end
        end
        ......@@ -13,7 +13,6 @@
        let_it_be(:security_finding_4) { build(:ci_reports_security_finding, uuid: nil) }
        let_it_be(:deduplicated_finding_uuids) { [security_finding_1.uuid, security_finding_3.uuid] }
        let_it_be(:security_scanner) { build(:ci_reports_security_scanner) }
        let_it_be(:multiple_artifact_allowed) { false }
        let_it_be(:report) do
        build(
        :ci_reports_security_report,
        ......@@ -23,9 +22,7 @@
        end
        describe '#execute' do
        let(:service_object) do
        described_class.new(security_scan, scanner, report, deduplicated_finding_uuids, multiple_artifact_allowed)
        end
        let(:service_object) { described_class.new(security_scan, scanner, report, deduplicated_finding_uuids) }
        subject(:store_findings) { service_object.execute }
        ......@@ -41,18 +38,6 @@
        it 'does not create new findings in database' do
        expect { store_findings }.not_to change(Security::Finding, :count)
        end
        context 'with multiple_artifact_allowed set to true' do
        let_it_be(:multiple_artifact_allowed) { true }
        it 'does not return error message' do
        expect(store_findings).to include(status: :success)
        end
        it 'does create new findings in database' do
        expect { store_findings }.to change(Security::Finding, :count).by(3)
        end
        end
        end
        context 'when the given security scan does not have any findings' do
        ......
        ......@@ -147,42 +147,6 @@
        expect(Security::StoreScanService).to have_received(:execute).with(artifact_3, empty_set, true).ordered
        end
        end
        context 'with multiple artifacts related to the same job' do
        using RSpec::Parameterized::TableSyntax
        let_it_be(:user) { create(:user) }
        let_it_be(:pipeline) { create(:ci_pipeline, user: user) }
        let_it_be(:build) { create(:ee_ci_build, :success, pipeline: pipeline) }
        let_it_be(:cyclonedx_artifact) { create(:ee_ci_job_artifact, :cyclonedx, job: build) }
        let_it_be(:dependency_scanning_artifact) { create(:ee_ci_job_artifact, :dependency_scanning, job: build) }
        let_it_be(:affected_package) { create(:pm_affected_package, purl_type: :gem, package_name: 'activesupport', affected_range: "<5.1.5") }
        let_it_be(:report_type) { :dependency_scanning }
        before do
        allow(Security::StoreScanService).to receive(:execute).and_call_original
        stub_feature_flags(dependency_scanning_for_pipelines_with_cyclonedx_reports: feature_flag_enabled)
        end
        where(:input_artifacts, :feature_flag_enabled, :cyclonedx_finding_count, :dependency_scanning_finding_count) do
        [ref(:cyclonedx_artifact)] | true | 1 | 0
        [ref(:dependency_scanning_artifact)] | true | 0 | 4
        [ref(:cyclonedx_artifact), ref(:dependency_scanning_artifact)] | true | 1 | 4
        [ref(:dependency_scanning_artifact), ref(:cyclonedx_artifact)] | true | 1 | 4
        [ref(:cyclonedx_artifact)] | false | 1 | 0
        [ref(:dependency_scanning_artifact)] | false | 0 | 4
        [ref(:cyclonedx_artifact), ref(:dependency_scanning_artifact)] | false | 0 | 4
        [ref(:dependency_scanning_artifact), ref(:cyclonedx_artifact)] | false | 0 | 4
        end
        with_them do
        let(:artifacts) { input_artifacts }
        it 'creates findings related to the artifacts' do
        expect { store_scan_group }.to change { Security::Finding.count }.by(cyclonedx_finding_count + dependency_scanning_finding_count)
        end
        end
        end
        end
        context 'when the artifacts are sast' do
        ......
        ......@@ -188,7 +188,6 @@
        describe 'executing `StoreFindingsService`' do
        let_it_be(:project) { artifact.project }
        let_it_be(:security_scanner) { artifact.security_report.primary_scanner }
        let_it_be(:multiple_artifacts_allowed) { false }
        context 'when there is already a vulnerability scanner' do
        let_it_be(:scanner) do
        ......@@ -199,53 +198,7 @@
        store_scan
        expect(Security::StoreFindingsService).to have_received(:execute).with(
        an_instance_of(Security::Scan),
        scanner,
        artifact.security_report,
        an_instance_of(Array),
        multiple_artifacts_allowed)
        end
        context 'with vulnerability scanner' do
        let_it_be(:multiple_artifacts_allowed) { true }
        let_it_be(:scanner_external_id) { Gitlab::VulnerabilityScanning::SecurityScanner::EXTERNAL_ID }
        let_it_be(:scanner) do
        create(:vulnerabilities_scanner, project: project, external_id: scanner_external_id)
        end
        before do
        allow(artifact.security_report.primary_scanner).to receive(:external_id).and_return(scanner_external_id)
        end
        it 'calls the `Security::StoreFindingsService` to store findings' do
        store_scan
        expect(Security::StoreFindingsService).to have_received(:execute).with(
        an_instance_of(Security::Scan),
        scanner,
        artifact.security_report,
        an_instance_of(Array),
        multiple_artifacts_allowed)
        end
        context 'with dependency_scanning_for_pipelines_with_cyclonedx_reports feature flag disabled' do
        let_it_be(:multiple_artifacts_allowed) { false }
        before do
        stub_feature_flags(dependency_scanning_for_pipelines_with_cyclonedx_reports: false)
        end
        it 'calls the `Security::StoreFindingsService` to store findings' do
        store_scan
        expect(Security::StoreFindingsService).to have_received(:execute).with(
        an_instance_of(Security::Scan),
        scanner,
        artifact.security_report,
        an_instance_of(Array),
        multiple_artifacts_allowed)
        end
        end
        an_instance_of(Security::Scan), scanner, artifact.security_report, an_instance_of(Array))
        end
        it 'does not create a new scanner' do
        ......@@ -260,11 +213,7 @@
        created_scanner = project.vulnerability_scanners.find_by(external_id: security_scanner.external_id)
        expect(Security::StoreFindingsService).to have_received(:execute).with(
        an_instance_of(Security::Scan),
        created_scanner,
        artifact.security_report,
        an_instance_of(Array),
        multiple_artifacts_allowed)
        an_instance_of(Security::Scan), created_scanner, artifact.security_report, an_instance_of(Array))
        end
        it 'creates a new scanner' do
        ......
        ......@@ -13,8 +13,7 @@ class Scanner
        "gemnasium-python" => 3,
        "bandit" => 1,
        "spotbugs" => 1,
        "semgrep" => 2,
        "gitlab-sbom-vulnerability-scanner" => 4
        "semgrep" => 2
        }.freeze
        attr_accessor :external_id, :name, :vendor, :version, :primary_identifiers
        ......
        ......@@ -110,9 +110,6 @@
        { external_id: 'spotbugs', name: 'foo', vendor: 'bar' } | { external_id: 'semgrep', name: 'foo', vendor: 'bar' } | -1
        { external_id: 'semgrep', name: 'foo', vendor: 'bar' } | { external_id: 'unknown', name: 'foo', vendor: 'bar' } | -1
        { external_id: 'gemnasium', name: 'foo', vendor: 'bar' } | { external_id: 'gemnasium', name: 'foo', vendor: nil } | 1
        { external_id: 'gitlab-sbom-vulnerability-scanner', name: 'foo', vendor: 'bar' } | { external_id: 'gemnasium', name: 'foo', vendor: 'bar' } | 1
        { external_id: 'gitlab-sbom-vulnerability-scanner', name: 'foo', vendor: 'bar' } | { external_id: 'gemnasium-python', name: 'foo', vendor: 'bar' } | 1
        { external_id: 'gitlab-sbom-vulnerability-scanner', name: 'foo', vendor: 'bar' } | { external_id: 'gemnasium-maven', name: 'foo', vendor: 'bar' } | 1
        end
        with_them do
        ......
        0% Loading or .
        You are about to add 0 people to the discussion. Proceed with caution.
        Finish editing this message first!
        Please register or to comment