Commit 0ffcdd71 authored by Can Eldem's avatar Can Eldem Committed by James Lopez

Present container scanning methods in merge request widget

Add new api point in merge request controller
Create new comparision class
parent 598f360a
...@@ -42,6 +42,10 @@ module EE ...@@ -42,6 +42,10 @@ module EE
reports_response(merge_request.compare_license_management_reports) reports_response(merge_request.compare_license_management_reports)
end end
def container_scanning_reports
reports_response(merge_request.compare_container_scanning_reports)
end
def metrics_reports def metrics_reports
reports_response(merge_request.compare_metrics_reports) reports_response(merge_request.compare_metrics_reports)
end end
......
...@@ -15,6 +15,7 @@ module EE ...@@ -15,6 +15,7 @@ module EE
LICENSE_MANAGEMENT_REPORT_FILE_TYPES = %w[license_management].freeze LICENSE_MANAGEMENT_REPORT_FILE_TYPES = %w[license_management].freeze
DEPENDENCY_LIST_REPORT_FILE_TYPES = %w[dependency_scanning].freeze DEPENDENCY_LIST_REPORT_FILE_TYPES = %w[dependency_scanning].freeze
METRICS_REPORT_FILE_TYPES = %w[metrics].freeze METRICS_REPORT_FILE_TYPES = %w[metrics].freeze
CONTAINER_SCANNING_REPORT_TYPES = %w[container_scanning].freeze
scope :not_expired, -> { where('expire_at IS NULL OR expire_at > ?', Time.current) } scope :not_expired, -> { where('expire_at IS NULL OR expire_at > ?', Time.current) }
scope :project_id_in, ->(ids) { joins(:project).merge(::Project.id_in(ids)) } scope :project_id_in, ->(ids) { joins(:project).merge(::Project.id_in(ids)) }
...@@ -33,6 +34,10 @@ module EE ...@@ -33,6 +34,10 @@ module EE
with_file_types(DEPENDENCY_LIST_REPORT_FILE_TYPES) with_file_types(DEPENDENCY_LIST_REPORT_FILE_TYPES)
end end
scope :container_scanning_reports, -> do
with_file_types(CONTAINER_SCANNING_REPORT_TYPES)
end
scope :metrics_reports, -> do scope :metrics_reports, -> do
with_file_types(METRICS_REPORT_FILE_TYPES) with_file_types(METRICS_REPORT_FILE_TYPES)
end end
......
...@@ -116,6 +116,18 @@ module EE ...@@ -116,6 +116,18 @@ module EE
actual_head_pipeline&.has_reports?(::Ci::JobArtifact.license_management_reports) actual_head_pipeline&.has_reports?(::Ci::JobArtifact.license_management_reports)
end end
def has_container_scanning_reports?
actual_head_pipeline&.has_reports?(::Ci::JobArtifact.container_scanning_reports)
end
def compare_container_scanning_reports
unless has_container_scanning_reports?
return { status: :error, status_reason: 'This merge request does not have container scanning reports' }
end
compare_reports(::Ci::CompareContainerScanningReportsService)
end
def compare_license_management_reports def compare_license_management_reports
unless has_license_management_reports? unless has_license_management_reports?
return { status: :error, status_reason: 'This merge request does not have license management reports' } return { status: :error, status_reason: 'This merge request does not have license management reports' }
......
# frozen_string_literal: true
class Vulnerabilities::OccurrenceReportsComparerEntity < Grape::Entity
expose :added, using: Vulnerabilities::OccurrenceReportEntity
expose :fixed, using: Vulnerabilities::OccurrenceReportEntity
expose :existing, using: Vulnerabilities::OccurrenceReportEntity
end
# frozen_string_literal: true
class Vulnerabilities::OccurrenceDiffSerializer < BaseSerializer
include WithPagination
entity Vulnerabilities::OccurrenceReportsComparerEntity
end
# frozen_string_literal: true
class Vulnerabilities::OccurrenceReportEntity < Grape::Entity
expose :report_type, :name, :severity, :confidence, :compare_key, :identifiers, :scanner, :project_fingerprint, :uuid, :metadata_version, :location
end
# frozen_string_literal: true
module Ci
class CompareContainerScanningReportsService < ::Ci::CompareReportsBaseService
def comparer_class
Gitlab::Ci::Reports::Security::ContainerScanningReportsComparer
end
def serializer_class
Vulnerabilities::OccurrenceDiffSerializer
end
def get_report(pipeline)
report = pipeline&.security_reports&.get_report('container_scanning')
raise report.error if report&.errored? # propagate error to base class's execute method
report
end
end
end
...@@ -17,3 +17,4 @@ ...@@ -17,3 +17,4 @@
window.gl.mrWidgetData.approvals_help_path = '#{help_page_path("user/project/merge_requests/merge_request_approvals")}'; window.gl.mrWidgetData.approvals_help_path = '#{help_page_path("user/project/merge_requests/merge_request_approvals")}';
window.gl.mrWidgetData.visual_review_app_available = '#{@project.feature_available?(:visual_review_app)}' === 'true'; window.gl.mrWidgetData.visual_review_app_available = '#{@project.feature_available?(:visual_review_app)}' === 'true';
window.gl.mrWidgetData.license_management_comparsion_path = '#{license_management_reports_project_merge_request_path(@project, @merge_request) if @project.feature_available?(:license_management)}' window.gl.mrWidgetData.license_management_comparsion_path = '#{license_management_reports_project_merge_request_path(@project, @merge_request) if @project.feature_available?(:license_management)}'
window.gl.mrWidgetData.container_scanning_comparsion_path = '#{container_scanning_reports_project_merge_request_path(@project, @merge_request) if @project.feature_available?(:container_scanning)}'
---
title: Present container scanning report comparison via API
merge_request: 14898
author:
type: changed
...@@ -69,6 +69,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do ...@@ -69,6 +69,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
member do member do
get :metrics_reports get :metrics_reports
get :license_management_reports get :license_management_reports
get :container_scanning_reports
end end
end end
......
# frozen_string_literal: true
module Gitlab
module Ci
module Reports
module Security
class ContainerScanningReportsComparer
include Gitlab::Utils::StrongMemoize
attr_reader :base_report, :head_report
def initialize(base_report, head_report)
@base_report = base_report || ::Gitlab::Ci::Reports::Security::Report.new('container_scanning', '')
@head_report = head_report
end
def added
strong_memoize(:added) do
head_report.occurrences - base_report.occurrences
end
end
def fixed
strong_memoize(:fixed) do
base_report.occurrences - head_report.occurrences
end
end
def existing
strong_memoize(:existing) do
base_report.occurrences & head_report.occurrences
end
end
end
end
end
end
end
...@@ -70,6 +70,12 @@ module Gitlab ...@@ -70,6 +70,12 @@ module Gitlab
other.location == location && other.location == location &&
other.primary_identifier == primary_identifier other.primary_identifier == primary_identifier
end end
# Array.difference (-) method uses hash and eq? methods to do comparison
def hash
compare_key.hash
end
alias_method :eql?, :== # eql? is necessary in some cases like array intersection alias_method :eql?, :== # eql? is necessary in some cases like array intersection
private private
......
...@@ -393,6 +393,91 @@ describe Projects::MergeRequestsController do ...@@ -393,6 +393,91 @@ describe Projects::MergeRequestsController do
end end
end end
describe 'GET #container_scanning_reports' do
let(:merge_request) { create(:ee_merge_request, :with_container_scanning_reports, source_project: project, author: create(:user)) }
let(:params) do
{
namespace_id: project.namespace.to_param,
project_id: project,
id: merge_request.iid
}
end
subject { get :container_scanning_reports, params: params, format: :json }
before do
allow_any_instance_of(::MergeRequest).to receive(:compare_reports)
.with(::Ci::CompareContainerScanningReportsService).and_return(comparison_status)
end
context 'when comparison is being processed' do
let(:comparison_status) { { status: :parsing } }
it 'sends polling interval' do
expect(::Gitlab::PollingInterval).to receive(:set_header)
subject
end
it 'returns 204 HTTP status' do
subject
expect(response).to have_gitlab_http_status(:no_content)
end
end
context 'when comparison is done' do
let(:comparison_status) { { status: :parsed, data: { added: [], fixed: [], existing: [] } } }
it 'does not send polling interval' do
expect(::Gitlab::PollingInterval).not_to receive(:set_header)
subject
end
it 'returns 200 HTTP status' do
subject
expect(response).to have_gitlab_http_status(:ok)
expect(json_response).to eq({ "added" => [], "fixed" => [], "existing" => [] })
end
end
context 'when user created corrupted vulnerability reports' do
let(:comparison_status) { { status: :error, status_reason: 'Failed to parse container scanning reports' } }
it 'does not send polling interval' do
expect(::Gitlab::PollingInterval).not_to receive(:set_header)
subject
end
it 'returns 400 HTTP status' do
subject
expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response).to eq({ 'status_reason' => 'Failed to parse container scanning reports' })
end
end
context 'when something went wrong on our system' do
let(:comparison_status) { {} }
it 'does not send polling interval' do
expect(::Gitlab::PollingInterval).not_to receive(:set_header)
subject
end
it 'returns 500 HTTP status' do
subject
expect(response).to have_gitlab_http_status(:internal_server_error)
expect(json_response).to eq({ 'status_reason' => 'Unknown error' })
end
end
end
describe 'GET #license_management_reports' do describe 'GET #license_management_reports' do
let(:merge_request) { create(:ee_merge_request, :with_license_management_reports, source_project: project, author: create(:user)) } let(:merge_request) { create(:ee_merge_request, :with_license_management_reports, source_project: project, author: create(:user)) }
let(:params) do let(:params) do
......
...@@ -48,6 +48,18 @@ FactoryBot.define do ...@@ -48,6 +48,18 @@ FactoryBot.define do
end end
end end
trait :container_scanning_feature_branch do
after(:build) do |build|
build.job_artifacts << create(:ee_ci_job_artifact, :container_scanning_feature_branch, job: build)
end
end
trait :corrupted_container_scanning_report do
after(:build) do |build|
build.job_artifacts << create(:ee_ci_job_artifact, :corrupted_container_scanning_report, job: build)
end
end
trait :license_management_feature_branch do trait :license_management_feature_branch do
after(:build) do |build| after(:build) do |build|
build.job_artifacts << create(:ee_ci_job_artifact, :license_management_feature_branch, job: build) build.job_artifacts << create(:ee_ci_job_artifact, :license_management_feature_branch, job: build)
......
...@@ -122,6 +122,26 @@ FactoryBot.define do ...@@ -122,6 +122,26 @@ FactoryBot.define do
end end
end end
trait :container_scanning_feature_branch do
file_format :raw
file_type :container_scanning
after(:build) do |artifact, _|
artifact.file = fixture_file_upload(
Rails.root.join('spec/fixtures/security-reports/feature-branch/gl-container-scanning-report.json'), 'application/json')
end
end
trait :corrupted_container_scanning_report do
file_format :raw
file_type :container_scanning
after(:build) do |artifact, evaluator|
artifact.file = fixture_file_upload(
Rails.root.join('spec/fixtures/trace/sample_trace'), 'application/json')
end
end
trait :dast do trait :dast do
file_format :raw file_format :raw
file_type :dast file_type :dast
......
...@@ -7,7 +7,7 @@ FactoryBot.define do ...@@ -7,7 +7,7 @@ FactoryBot.define do
config_source :webide_source config_source :webide_source
end end
%i[license_management dependency_list dependency_scanning sast].each do |report_type| %i[license_management dependency_list dependency_scanning sast container_scanning].each do |report_type|
trait "with_#{report_type}_report".to_sym do trait "with_#{report_type}_report".to_sym do
status :success status :success
...@@ -17,6 +17,22 @@ FactoryBot.define do ...@@ -17,6 +17,22 @@ FactoryBot.define do
end end
end end
trait :with_container_scanning_feature_branch do
status :success
after(:build) do |pipeline, evaluator|
pipeline.builds << build(:ee_ci_build, :container_scanning_feature_branch, pipeline: pipeline, project: pipeline.project)
end
end
trait :with_corrupted_container_scanning_report do
status :success
after(:build) do |pipeline, evaluator|
pipeline.builds << build(:ee_ci_build, :corrupted_container_scanning_report, pipeline: pipeline, project: pipeline.project)
end
end
trait :with_license_management_feature_branch do trait :with_license_management_feature_branch do
status :success status :success
......
...@@ -61,6 +61,18 @@ FactoryBot.define do ...@@ -61,6 +61,18 @@ FactoryBot.define do
end end
end end
trait :with_container_scanning_reports do
after(:build) do |merge_request|
merge_request.head_pipeline = build(
:ee_ci_pipeline,
:success,
:with_container_scanning_report,
project: merge_request.source_project,
ref: merge_request.source_branch,
sha: merge_request.diff_head_sha)
end
end
trait :with_metrics_reports do trait :with_metrics_reports do
after(:build) do |merge_request| after(:build) do |merge_request|
merge_request.head_pipeline = build( merge_request.head_pipeline = build(
......
...@@ -139,6 +139,27 @@ describe MergeRequest do ...@@ -139,6 +139,27 @@ describe MergeRequest do
end end
end end
describe '#has_container_scanning_reports?' do
subject { merge_request.has_container_scanning_reports? }
let(:project) { create(:project, :repository) }
before do
stub_licensed_features(container_scanning: true)
end
context 'when head pipeline has container scannning reports' do
let(:merge_request) { create(:ee_merge_request, :with_container_scanning_reports, source_project: project) }
it { is_expected.to be_truthy }
end
context 'when head pipeline does not have container scanning reports' do
let(:merge_request) { create(:ee_merge_request, source_project: project) }
it { is_expected.to be_falsey }
end
end
describe '#has_metrics_reports?' do describe '#has_metrics_reports?' do
subject { merge_request.has_metrics_reports? } subject { merge_request.has_metrics_reports? }
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
...@@ -160,6 +181,65 @@ describe MergeRequest do ...@@ -160,6 +181,65 @@ describe MergeRequest do
end end
end end
describe '#compare_container_scanning_reports' do
subject { merge_request.compare_container_scanning_reports }
let(:project) { create(:project, :repository) }
let(:merge_request) { create(:merge_request, source_project: project) }
let!(:base_pipeline) do
create(:ee_ci_pipeline,
:with_container_scanning_report,
project: project,
ref: merge_request.target_branch,
sha: merge_request.diff_base_sha)
end
before do
merge_request.update!(head_pipeline_id: head_pipeline.id)
end
context 'when head pipeline has container scanning reports' do
let!(:head_pipeline) do
create(:ee_ci_pipeline,
:with_container_scanning_report,
project: project,
ref: merge_request.source_branch,
sha: merge_request.diff_head_sha)
end
context 'when reactive cache worker is parsing asynchronously' do
it 'returns status' do
expect(subject[:status]).to eq(:parsing)
end
end
context 'when reactive cache worker is inline' do
before do
synchronous_reactive_cache(merge_request)
end
it 'returns status and data' do
expect_any_instance_of(Ci::CompareContainerScanningReportsService)
.to receive(:execute).with(base_pipeline, head_pipeline).and_call_original
subject
end
context 'when cached results is not latest' do
before do
allow_any_instance_of(Ci::CompareContainerScanningReportsService)
.to receive(:latest?).and_return(false)
end
it 'raises and InvalidateReactiveCache error' do
expect { subject }.to raise_error(ReactiveCaching::InvalidateReactiveCache)
end
end
end
end
end
describe '#compare_license_management_reports' do describe '#compare_license_management_reports' do
subject { merge_request.compare_license_management_reports } subject { merge_request.compare_license_management_reports }
......
# frozen_string_literal: true
require 'spec_helper'
describe Vulnerabilities::OccurrenceReportsComparerEntity do
let!(:base_pipeline) { create(:ee_ci_pipeline, :with_container_scanning_report) }
let!(:head_pipeline) { create(:ee_ci_pipeline, :with_container_scanning_feature_branch) }
let(:base_report) { base_pipeline.security_reports.get_report('container_scanning')}
let(:head_report) { head_pipeline.security_reports.get_report('container_scanning')}
let(:comparer) { Gitlab::Ci::Reports::Security::ContainerScanningReportsComparer.new(base_report, head_report) }
let(:entity) { described_class.new(comparer) }
before do
stub_licensed_features(container_scanning: true)
end
describe '#as_json' do
subject { entity.as_json }
it 'contains the added existing and fixed vulnerabilities for container scanning' do
expect(subject.keys).to match_array([:added, :existing, :fixed])
end
end
end
require 'spec_helper'
describe Ci::CompareContainerScanningReportsService do
let(:service) { described_class.new(project) }
let(:project) { create(:project, :repository) }
before do
stub_licensed_features(container_scanning: true)
end
describe '#execute' do
subject { service.execute(base_pipeline, head_pipeline) }
context 'when head pipeline has container scanning reports' do
let!(:base_pipeline) { create(:ee_ci_pipeline) }
let!(:head_pipeline) { create(:ee_ci_pipeline, :with_container_scanning_report, project: project) }
it 'reports new licenses' do
expect(subject[:status]).to eq(:parsed)
expect(subject[:data]['added'].count).to eq(8)
expect(subject[:data]['existing'].count).to eq(0)
expect(subject[:data]['fixed'].count).to eq(0)
end
end
context 'when base and head pipelines have container scanning reports' do
let!(:base_pipeline) { create(:ee_ci_pipeline, :with_container_scanning_report, project: project) }
let!(:head_pipeline) { create(:ee_ci_pipeline, :with_container_scanning_feature_branch, project: project) }
it 'reports status as parsed' do
expect(subject[:status]).to eq(:parsed)
end
it 'reports new vulnerability' do
expect(subject[:data]['added'].count).to eq(1)
expect(subject[:data]['added']).to include(a_hash_including('compare_key' => 'CVE-2017-15650'))
end
it 'reports existing container vulenerabilities' do
expect(subject[:data]['existing'].count).to eq(0)
end
it 'reports fixed container scanning vulnerabilities' do
expect(subject[:data]['fixed'].count).to eq(8)
compare_keys = subject[:data]['fixed'].map { |t| t['compare_key'] }
expected_keys = %w(CVE-2017-16997 CVE-2017-18269 CVE-2018-1000001 CVE-2016-10228 CVE-2010-4052 CVE-2018-18520 CVE-2018-16869 CVE-2018-18311)
expect(compare_keys - expected_keys).to eq([])
end
end
context 'when head pipeline has corrupted container scanning vulnerability reports' do
let!(:base_pipeline) { nil }
let!(:head_pipeline) { create(:ee_ci_pipeline, :with_corrupted_container_scanning_report, project: project) }
it 'returns status and error message' do
expect(subject[:status]).to eq(:error)
expect(subject[:status_reason]).to include('JSON parsing failed')
end
end
end
end
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