Commit ac8fab3f authored by Dylan Griffith's avatar Dylan Griffith

Merge branch '338665-code-cleanup' into 'master'

Remove pipeline association from Vulnerabilities::Finding

See merge request gitlab-org/gitlab!73115
parents c1953919 84b9c5bc
---
name: finding_ci_pipeline_disable_joins
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/70216
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/338665
milestone: '14.3'
type: development
group: group::threat insights
default_enabled: true
...@@ -34,7 +34,6 @@ module Vulnerabilities ...@@ -34,7 +34,6 @@ module Vulnerabilities
has_many :remediations, through: :finding_remediations has_many :remediations, through: :finding_remediations
has_many :finding_pipelines, class_name: 'Vulnerabilities::FindingPipeline', inverse_of: :finding, foreign_key: 'occurrence_id' has_many :finding_pipelines, class_name: 'Vulnerabilities::FindingPipeline', inverse_of: :finding, foreign_key: 'occurrence_id'
has_many :pipelines, through: :finding_pipelines, class_name: 'Ci::Pipeline', disable_joins: -> { ::Feature.enabled?(:finding_ci_pipeline_disable_joins, default_enabled: :yaml) }
has_many :signatures, class_name: 'Vulnerabilities::FindingSignature', inverse_of: :finding has_many :signatures, class_name: 'Vulnerabilities::FindingSignature', inverse_of: :finding
......
...@@ -64,8 +64,7 @@ RSpec.describe Projects::IssuesController do ...@@ -64,8 +64,7 @@ RSpec.describe Projects::IssuesController do
render_views render_views
context 'when a vulnerability_id is provided' do context 'when a vulnerability_id is provided' do
let(:pipeline) { create(:ci_pipeline, project: project) } let(:finding) { create(:vulnerabilities_finding, :with_pipeline) }
let(:finding) { create(:vulnerabilities_finding, pipelines: [pipeline]) }
let(:vulnerability) { create(:vulnerability, project: project, findings: [finding]) } let(:vulnerability) { create(:vulnerability, project: project, findings: [finding]) }
let(:vulnerability_field) { "<input type=\"hidden\" name=\"vulnerability_id\" id=\"vulnerability_id\" value=\"#{vulnerability.id}\" />" } let(:vulnerability_field) { "<input type=\"hidden\" name=\"vulnerability_id\" id=\"vulnerability_id\" value=\"#{vulnerability.id}\" />" }
...@@ -98,8 +97,7 @@ RSpec.describe Projects::IssuesController do ...@@ -98,8 +97,7 @@ RSpec.describe Projects::IssuesController do
end end
context 'when created from a vulnerability' do context 'when created from a vulnerability' do
let(:pipeline) { create(:ci_pipeline, project: project) } let(:finding) { create(:vulnerabilities_finding, :with_pipeline) }
let(:finding) { create(:vulnerabilities_finding, pipelines: [pipeline]) }
let(:vulnerability) { create(:vulnerability, project: project, findings: [finding]) } let(:vulnerability) { create(:vulnerability, project: project, findings: [finding]) }
before do before do
...@@ -188,8 +186,7 @@ RSpec.describe Projects::IssuesController do ...@@ -188,8 +186,7 @@ RSpec.describe Projects::IssuesController do
render_views render_views
context 'when a vulnerability_id is provided' do context 'when a vulnerability_id is provided' do
let(:pipeline) { create(:ci_pipeline, project: project) } let(:finding) { create(:vulnerabilities_finding, :with_pipeline) }
let(:finding) { create(:vulnerabilities_finding, pipelines: [pipeline]) }
let(:vulnerability) { create(:vulnerability, project: project, findings: [finding]) } let(:vulnerability) { create(:vulnerability, project: project, findings: [finding]) }
let(:vulnerability_field) { "<input type=\"hidden\" name=\"vulnerability_id\" id=\"vulnerability_id\" value=\"#{vulnerability.id}\" />" } let(:vulnerability_field) { "<input type=\"hidden\" name=\"vulnerability_id\" id=\"vulnerability_id\" value=\"#{vulnerability.id}\" />" }
......
...@@ -26,7 +26,7 @@ RSpec.describe Projects::Security::VulnerabilitiesController do ...@@ -26,7 +26,7 @@ RSpec.describe Projects::Security::VulnerabilitiesController do
end end
context "when there's an attached pipeline" do context "when there's an attached pipeline" do
let_it_be(:finding) { create(:vulnerabilities_finding, vulnerability: vulnerability, pipelines: [pipeline]) } let_it_be(:finding) { create(:vulnerabilities_finding, :with_pipeline, vulnerability: vulnerability) }
it 'renders the vulnerability page' do it 'renders the vulnerability page' do
show_vulnerability show_vulnerability
......
...@@ -6,7 +6,7 @@ RSpec.describe VulnerabilitiesHelper do ...@@ -6,7 +6,7 @@ RSpec.describe VulnerabilitiesHelper do
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project, :repository, :public) } let_it_be(:project) { create(:project, :repository, :public) }
let_it_be(:pipeline) { create(:ci_pipeline, :success, project: project) } let_it_be(:pipeline) { create(:ci_pipeline, :success, project: project) }
let_it_be(:finding) { create(:vulnerabilities_finding, pipelines: [pipeline], project: project, severity: :high) } let_it_be(:finding) { create(:vulnerabilities_finding, :with_pipeline, project: project, severity: :high) }
let(:vulnerability) { create(:vulnerability, title: "My vulnerability", project: project, findings: [finding]) } let(:vulnerability) { create(:vulnerability, title: "My vulnerability", project: project, findings: [finding]) }
......
...@@ -27,7 +27,7 @@ RSpec.describe Gitlab::DataBuilder::Vulnerability do ...@@ -27,7 +27,7 @@ RSpec.describe Gitlab::DataBuilder::Vulnerability do
let(:finding) do let(:finding) do
build(:vulnerabilities_finding, build(:vulnerabilities_finding,
pipelines: [pipeline], :with_pipeline,
identifiers: [identifier], identifiers: [identifier],
primary_identifier: identifier, primary_identifier: identifier,
scanner: scanner, scanner: scanner,
......
...@@ -816,7 +816,7 @@ RSpec.describe Vulnerability do ...@@ -816,7 +816,7 @@ RSpec.describe Vulnerability do
describe '#blob_path' do describe '#blob_path' do
let_it_be(:vulnerability) { create(:vulnerability) } let_it_be(:vulnerability) { create(:vulnerability) }
let_it_be(:pipeline) { create(:ci_pipeline) } let_it_be(:pipeline) { create(:ci_pipeline) }
let_it_be(:finding) { create(:vulnerabilities_finding, pipelines: [pipeline], vulnerability: vulnerability) } let_it_be(:finding) { create(:vulnerabilities_finding, :with_pipeline, vulnerability: vulnerability) }
subject { vulnerability.blob_path } subject { vulnerability.blob_path }
......
...@@ -18,7 +18,6 @@ RSpec.describe Vulnerabilities::Finding do ...@@ -18,7 +18,6 @@ RSpec.describe Vulnerabilities::Finding do
it { is_expected.to belong_to(:primary_identifier).class_name('Vulnerabilities::Identifier') } it { is_expected.to belong_to(:primary_identifier).class_name('Vulnerabilities::Identifier') }
it { is_expected.to belong_to(:scanner).class_name('Vulnerabilities::Scanner') } it { is_expected.to belong_to(:scanner).class_name('Vulnerabilities::Scanner') }
it { is_expected.to belong_to(:vulnerability).inverse_of(:findings) } it { is_expected.to belong_to(:vulnerability).inverse_of(:findings) }
it { is_expected.to have_many(:pipelines).class_name('Ci::Pipeline') }
it { is_expected.to have_many(:finding_pipelines).class_name('Vulnerabilities::FindingPipeline').with_foreign_key('occurrence_id') } it { is_expected.to have_many(:finding_pipelines).class_name('Vulnerabilities::FindingPipeline').with_foreign_key('occurrence_id') }
it { is_expected.to have_many(:identifiers).class_name('Vulnerabilities::Identifier') } it { is_expected.to have_many(:identifiers).class_name('Vulnerabilities::Identifier') }
it { is_expected.to have_many(:finding_identifiers).class_name('Vulnerabilities::FindingIdentifier').with_foreign_key('occurrence_id') } it { is_expected.to have_many(:finding_identifiers).class_name('Vulnerabilities::FindingIdentifier').with_foreign_key('occurrence_id') }
......
...@@ -9,7 +9,7 @@ RSpec.describe VulnerabilityPresenter do ...@@ -9,7 +9,7 @@ RSpec.describe VulnerabilityPresenter do
{ type: 'sast', status: 'success', start_time: 'placeholder', end_time: 'placeholder' } { type: 'sast', status: 'success', start_time: 'placeholder', end_time: 'placeholder' }
end end
let_it_be(:finding) { create(:vulnerabilities_finding, :with_secret_detection, pipelines: [pipeline], project: project, description: 'Finding Description') } let_it_be(:finding) { create(:vulnerabilities_finding, :with_secret_detection, :with_pipeline, project: project, description: 'Finding Description') }
let_it_be(:finding2) { create(:vulnerabilities_finding, :with_secret_detection, scan: sast_scan) } let_it_be(:finding2) { create(:vulnerabilities_finding, :with_secret_detection, scan: sast_scan) }
subject { described_class.new(finding.vulnerability) } subject { described_class.new(finding.vulnerability) }
...@@ -39,7 +39,7 @@ RSpec.describe VulnerabilityPresenter do ...@@ -39,7 +39,7 @@ RSpec.describe VulnerabilityPresenter do
end end
describe '#remediations' do describe '#remediations' do
let(:finding) { create(:vulnerabilities_finding, :with_secret_detection, :with_remediation, pipelines: [pipeline], project: project) } let(:finding) { create(:vulnerabilities_finding, :with_secret_detection, :with_remediation, :with_pipeline, project: project) }
it 'returns remediations' do it 'returns remediations' do
expect(subject.remediations.count).to be(1) expect(subject.remediations.count).to be(1)
......
...@@ -8,7 +8,7 @@ RSpec.describe 'Creating an External Issue Link' do ...@@ -8,7 +8,7 @@ RSpec.describe 'Creating an External Issue Link' do
let_it_be(:current_user) { create(:user) } let_it_be(:current_user) { create(:user) }
let_it_be(:project) { create(:project) } let_it_be(:project) { create(:project) }
let_it_be(:pipeline) { create(:ci_pipeline, :success, project: project) } let_it_be(:pipeline) { create(:ci_pipeline, :success, project: project) }
let_it_be(:finding) { create(:vulnerabilities_finding, pipelines: [pipeline], project: project, severity: :high) } let_it_be(:finding) { create(:vulnerabilities_finding, :with_pipeline, project: project, severity: :high) }
let_it_be(:vulnerability) { create(:vulnerability, title: 'My vulnerability', project: project, findings: [finding]) } let_it_be(:vulnerability) { create(:vulnerability, title: 'My vulnerability', project: project, findings: [finding]) }
let_it_be(:external_provider) { 'jira' } let_it_be(:external_provider) { 'jira' }
......
...@@ -202,9 +202,9 @@ RSpec.describe 'Query.vulnerabilities.location' do ...@@ -202,9 +202,9 @@ RSpec.describe 'Query.vulnerabilities.location' do
let_it_be(:finding) do let_it_be(:finding) do
create( create(
:vulnerabilities_finding, :vulnerabilities_finding,
:with_pipeline,
vulnerability: vulnerability, vulnerability: vulnerability,
raw_metadata: metadata.to_json, raw_metadata: metadata.to_json
pipelines: [pipeline]
) )
end end
...@@ -241,9 +241,9 @@ RSpec.describe 'Query.vulnerabilities.location' do ...@@ -241,9 +241,9 @@ RSpec.describe 'Query.vulnerabilities.location' do
let_it_be(:finding) do let_it_be(:finding) do
create( create(
:vulnerabilities_finding, :vulnerabilities_finding,
:with_pipeline,
vulnerability: vulnerability, vulnerability: vulnerability,
raw_metadata: metadata.to_json, raw_metadata: metadata.to_json
pipelines: [pipeline]
) )
end end
...@@ -281,9 +281,9 @@ RSpec.describe 'Query.vulnerabilities.location' do ...@@ -281,9 +281,9 @@ RSpec.describe 'Query.vulnerabilities.location' do
let_it_be(:finding) do let_it_be(:finding) do
create( create(
:vulnerabilities_finding, :vulnerabilities_finding,
:with_pipeline,
vulnerability: vulnerability, vulnerability: vulnerability,
raw_metadata: metadata.to_json, raw_metadata: metadata.to_json
pipelines: [pipeline]
) )
end end
......
...@@ -41,11 +41,14 @@ RSpec.describe Security::AutoFixService do ...@@ -41,11 +41,14 @@ RSpec.describe Security::AutoFixService do
let!(:vulnerability) do let!(:vulnerability) do
create(:vulnerabilities_finding_with_remediation, :yarn_remediation, :identifier, create(:vulnerabilities_finding_with_remediation, :yarn_remediation, :identifier,
project: project, project: project,
pipelines: [pipeline],
report_type: :dependency_scanning, report_type: :dependency_scanning,
summary: 'Test remediation') summary: 'Test remediation')
end end
before do
create(:vulnerabilities_finding_pipeline, finding: vulnerability, pipeline: pipeline)
end
it 'creates MR' do it 'creates MR' do
result = execute_service result = execute_service
...@@ -109,7 +112,7 @@ RSpec.describe Security::AutoFixService do ...@@ -109,7 +112,7 @@ RSpec.describe Security::AutoFixService do
context 'without remediations' do context 'without remediations' do
before do before do
create(:vulnerabilities_finding, report_type: :dependency_scanning, pipelines: [pipeline], project: project) create(:vulnerabilities_finding, :with_pipeline, report_type: :dependency_scanning, project: project)
end end
it 'does not create merge request' do it 'does not create merge request' do
......
...@@ -309,7 +309,6 @@ RSpec.describe Security::StoreReportService, '#execute', :snowplow do ...@@ -309,7 +309,6 @@ RSpec.describe Security::StoreReportService, '#execute', :snowplow do
let!(:finding) do let!(:finding) do
created_finding = create(:vulnerabilities_finding, created_finding = create(:vulnerabilities_finding,
pipelines: [pipeline],
identifiers: [identifier], identifiers: [identifier],
primary_identifier: identifier, primary_identifier: identifier,
scanner: scanner, scanner: scanner,
...@@ -340,7 +339,6 @@ RSpec.describe Security::StoreReportService, '#execute', :snowplow do ...@@ -340,7 +339,6 @@ RSpec.describe Security::StoreReportService, '#execute', :snowplow do
let!(:finding_with_uuidv5) do let!(:finding_with_uuidv5) do
create(:vulnerabilities_finding, create(:vulnerabilities_finding,
pipelines: [pipeline],
identifiers: [different_identifier], identifiers: [different_identifier],
primary_identifier: different_identifier, primary_identifier: different_identifier,
scanner: scanner, scanner: scanner,
...@@ -356,7 +354,6 @@ RSpec.describe Security::StoreReportService, '#execute', :snowplow do ...@@ -356,7 +354,6 @@ RSpec.describe Security::StoreReportService, '#execute', :snowplow do
let!(:finding_with_wrong_uuidv5) do let!(:finding_with_wrong_uuidv5) do
create(:vulnerabilities_finding, create(:vulnerabilities_finding,
pipelines: [pipeline],
identifiers: [identifier_of_corrupted_finding], identifiers: [identifier_of_corrupted_finding],
primary_identifier: identifier_of_corrupted_finding, primary_identifier: identifier_of_corrupted_finding,
scanner: scanner, scanner: scanner,
...@@ -463,7 +460,7 @@ RSpec.describe Security::StoreReportService, '#execute', :snowplow do ...@@ -463,7 +460,7 @@ RSpec.describe Security::StoreReportService, '#execute', :snowplow do
context 'when the existing vulnerability requires manual resolution' do context 'when the existing vulnerability requires manual resolution' do
let(:trait) { :secret_detection } let(:trait) { :secret_detection }
let!(:finding) { create(:vulnerabilities_finding, :with_secret_detection, project: project, pipelines: [pipeline]) } let!(:finding) { create(:vulnerabilities_finding, :with_secret_detection, project: project) }
it 'wont mark the vulnerability as resolved on default branch' do it 'wont mark the vulnerability as resolved on default branch' do
expect { subject }.not_to change { finding.vulnerability.reload.resolved_on_default_branch } expect { subject }.not_to change { finding.vulnerability.reload.resolved_on_default_branch }
...@@ -569,7 +566,8 @@ RSpec.describe Security::StoreReportService, '#execute', :snowplow do ...@@ -569,7 +566,8 @@ RSpec.describe Security::StoreReportService, '#execute', :snowplow do
end end
context 'with existing data from same pipeline' do context 'with existing data from same pipeline' do
let!(:finding) { create(:vulnerabilities_finding, project: project, pipelines: [pipeline]) } let(:finding) { create(:vulnerabilities_finding, :with_pipeline, project: project) }
let!(:finding_pipeline) { create(:vulnerabilities_finding_pipeline, finding: finding, pipeline: pipeline) }
let(:trait) { :sast } let(:trait) { :sast }
it 'skips report' do it 'skips report' do
......
...@@ -6,7 +6,7 @@ RSpec.describe VulnerabilityExternalIssueLinks::CreateService do ...@@ -6,7 +6,7 @@ RSpec.describe VulnerabilityExternalIssueLinks::CreateService do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:project) { create(:project) } let(:project) { create(:project) }
let(:pipeline) { create(:ci_pipeline, :success, project: project) } let(:pipeline) { create(:ci_pipeline, :success, project: project) }
let(:finding) { create(:vulnerabilities_finding, pipelines: [pipeline], project: project, severity: :high) } let(:finding) { create(:vulnerabilities_finding, :with_pipeline, project: project, severity: :high) }
let(:vulnerability) { create(:vulnerability, title: 'My vulnerability', project: project, findings: [finding]) } let(:vulnerability) { create(:vulnerability, title: 'My vulnerability', project: project, findings: [finding]) }
let(:external_provider) { 'jira' } let(:external_provider) { 'jira' }
let(:service) { described_class.new(user, vulnerability, external_provider) } let(:service) { described_class.new(user, vulnerability, external_provider) }
......
...@@ -13,7 +13,7 @@ RSpec.describe ScanSecurityReportSecretsWorker do ...@@ -13,7 +13,7 @@ RSpec.describe ScanSecurityReportSecretsWorker do
let(:revocation_key_type) { 'gitleaks_rule_id_aws' } let(:revocation_key_type) { 'gitleaks_rule_id_aws' }
let(:vulnerability) do let(:vulnerability) do
create(:vulnerabilities_finding, :with_secret_detection, pipelines: [pipeline], project: project) create(:vulnerabilities_finding, :with_secret_detection, project: project)
end end
subject(:worker) { described_class.new } subject(:worker) { described_class.new }
...@@ -24,6 +24,7 @@ RSpec.describe ScanSecurityReportSecretsWorker do ...@@ -24,6 +24,7 @@ RSpec.describe ScanSecurityReportSecretsWorker do
location: { file: file, location: { file: file,
start_line: 40, end_line: 45 }, start_line: 40, end_line: 45 },
identifiers: [{ type: identifier_type, name: 'Gitleaks rule ID AWS', value: identifier_value }] }.to_json) identifiers: [{ type: identifier_type, name: 'Gitleaks rule ID AWS', value: identifier_value }] }.to_json)
create(:vulnerabilities_finding_pipeline, finding: vulnerability, pipeline: pipeline)
end end
describe '#perform' do describe '#perform' do
......
...@@ -9,7 +9,8 @@ RSpec.describe StoreSecurityReportsWorker do ...@@ -9,7 +9,8 @@ RSpec.describe StoreSecurityReportsWorker do
describe '#secret_detection_vulnerability_found?' do describe '#secret_detection_vulnerability_found?' do
before do before do
create(:vulnerabilities_finding, :with_secret_detection, pipelines: [secret_detection_pipeline], project: secret_detection_pipeline.project) finding = create(:vulnerabilities_finding, :with_secret_detection, project: secret_detection_pipeline.project)
create(:vulnerabilities_finding_pipeline, finding: finding, pipeline: secret_detection_pipeline)
end end
specify { expect(described_class.new.send(:secret_detection_vulnerability_found?, secret_detection_pipeline)).to be(true) } specify { expect(described_class.new.send(:secret_detection_vulnerability_found?, secret_detection_pipeline)).to be(true) }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment