summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorOlivier Gonzalez <ogonzalez@gitlab.com>2018-09-27 21:15:08 +0000
committerKamil Trzciński <ayufan@ayufan.eu>2018-09-27 21:15:08 +0000
commitcfedc0a9f4732afbf39fdf96e9b6a8598faeba5f (patch)
tree001b6eb5fd448bc6389842bbe1d03b8587a6b55b
parent79498893832db7a88e07d8f3e7a629944c80705b (diff)
downloadgitlab-ce-cfedc0a9f4732afbf39fdf96e9b6a8598faeba5f.tar.gz
Extend reports to support security features
-rw-r--r--app/models/ci/build.rb59
-rw-r--r--app/models/ci/job_artifact.rb42
-rw-r--r--app/workers/expire_build_instance_artifacts_worker.rb2
-rw-r--r--changelogs/unreleased/6717_extend_reports_for_security_products.yml5
-rw-r--r--lib/gitlab/ci/config/entry/reports.rb8
-rw-r--r--lib/gitlab/ci/parsers.rb9
-rw-r--r--lib/gitlab/ci/parsers/junit.rb66
-rw-r--r--lib/gitlab/ci/parsers/test.rb21
-rw-r--r--lib/gitlab/ci/parsers/test/junit.rb70
-rw-r--r--spec/factories/ci/job_artifacts.rb27
-rw-r--r--spec/lib/gitlab/ci/config/entry/reports_spec.rb60
-rw-r--r--spec/lib/gitlab/ci/parsers/test/junit_spec.rb (renamed from spec/lib/gitlab/ci/parsers/junit_spec.rb)2
-rw-r--r--spec/lib/gitlab/ci/parsers/test_spec.rb (renamed from spec/lib/gitlab/ci/parsers_spec.rb)4
-rw-r--r--spec/models/ci/build_spec.rb76
-rw-r--r--spec/models/ci/job_artifact_spec.rb62
-rw-r--r--spec/presenters/ci/build_runner_presenter_spec.rb49
-rw-r--r--spec/requests/api/jobs_spec.rb2
-rw-r--r--spec/services/ci/retry_build_service_spec.rb18
18 files changed, 379 insertions, 203 deletions
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 63aaa0f7bcc..3dadb95443a 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -68,8 +68,12 @@ module Ci
'', Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.job_id').archive)
end
+ scope :with_existing_job_artifacts, ->(query) do
+ where('EXISTS (?)', ::Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.job_id').merge(query))
+ end
+
scope :with_archived_trace, ->() do
- where('EXISTS (?)', Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.job_id').trace)
+ with_existing_job_artifacts(Ci::JobArtifact.trace)
end
scope :without_archived_trace, ->() do
@@ -77,10 +81,12 @@ module Ci
end
scope :with_test_reports, ->() do
- includes(:job_artifacts_junit) # Prevent N+1 problem when iterating each ci_job_artifact row
- .where('EXISTS (?)', Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.job_id').test_reports)
+ with_existing_job_artifacts(Ci::JobArtifact.test_reports)
+ .eager_load_job_artifacts
end
+ scope :eager_load_job_artifacts, -> { includes(:job_artifacts) }
+
scope :with_artifacts_stored_locally, -> { with_artifacts_archive.where(artifacts_file_store: [nil, LegacyArtifactUploader::Store::LOCAL]) }
scope :with_archived_trace_stored_locally, -> { with_archived_trace.where(artifacts_file_store: [nil, LegacyArtifactUploader::Store::LOCAL]) }
scope :with_artifacts_not_expired, ->() { with_artifacts_archive.where('artifacts_expire_at IS NULL OR artifacts_expire_at > ?', Time.now) }
@@ -404,8 +410,8 @@ module Ci
trace.exist?
end
- def has_test_reports?
- job_artifacts.test_reports.any?
+ def has_job_artifacts?
+ job_artifacts.any?
end
def has_old_trace?
@@ -469,28 +475,23 @@ module Ci
end
end
- def erase_artifacts!
- remove_artifacts_file!
- remove_artifacts_metadata!
- save
- end
-
- def erase_test_reports!
- # TODO: Use fast_destroy_all in the context of https://gitlab.com/gitlab-org/gitlab-ce/issues/35240
- job_artifacts_junit&.destroy
+ # and use that for `ExpireBuildInstanceArtifactsWorker`?
+ def erase_erasable_artifacts!
+ job_artifacts.erasable.destroy_all # rubocop: disable DestroyAll
+ erase_old_artifacts!
end
def erase(opts = {})
return false unless erasable?
- erase_artifacts!
- erase_test_reports!
+ job_artifacts.destroy_all # rubocop: disable DestroyAll
+ erase_old_artifacts!
erase_trace!
update_erased!(opts[:erased_by])
end
def erasable?
- complete? && (artifacts? || has_test_reports? || has_trace?)
+ complete? && (artifacts? || has_job_artifacts? || has_trace?)
end
def erased?
@@ -648,8 +649,8 @@ module Ci
def collect_test_reports!(test_reports)
test_reports.get_suite(group_name).tap do |test_suite|
- each_test_report do |file_type, blob|
- Gitlab::Ci::Parsers.fabricate!(file_type).parse!(blob, test_suite)
+ each_report(Ci::JobArtifact::TEST_REPORT_FILE_TYPES) do |file_type, blob|
+ Gitlab::Ci::Parsers::Test.fabricate!(file_type).parse!(blob, test_suite)
end
end
end
@@ -669,6 +670,13 @@ module Ci
private
+ def erase_old_artifacts!
+ # TODO: To be removed once we get rid of
+ remove_artifacts_file!
+ remove_artifacts_metadata!
+ save
+ end
+
def successful_deployment_status
if success? && last_deployment&.last?
return :last
@@ -679,14 +687,19 @@ module Ci
:creating
end
- def each_test_report
- Ci::JobArtifact::TEST_REPORT_FILE_TYPES.each do |file_type|
- public_send("job_artifacts_#{file_type}").each_blob do |blob| # rubocop:disable GitlabSecurity/PublicSend
- yield file_type, blob
+ def each_report(report_types)
+ job_artifacts_for_types(report_types).each do |report_artifact|
+ report_artifact.each_blob do |blob|
+ yield report_artifact.file_type, blob
end
end
end
+ def job_artifacts_for_types(report_types)
+ # Use select to leverage cached associations and avoid N+1 queries
+ job_artifacts.select { |artifact| artifact.file_type.in?(report_types) }
+ end
+
def update_artifacts_size
self.artifacts_size = legacy_artifacts_file&.size
end
diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb
index 93fc1b145b2..259d85e2fe5 100644
--- a/app/models/ci/job_artifact.rb
+++ b/app/models/ci/job_artifact.rb
@@ -9,8 +9,28 @@ module Ci
NotSupportedAdapterError = Class.new(StandardError)
TEST_REPORT_FILE_TYPES = %w[junit].freeze
- DEFAULT_FILE_NAMES = { junit: 'junit.xml' }.freeze
- TYPE_AND_FORMAT_PAIRS = { archive: :zip, metadata: :gzip, trace: :raw, junit: :gzip }.freeze
+ NON_ERASABLE_FILE_TYPES = %w[trace].freeze
+ DEFAULT_FILE_NAMES = {
+ archive: nil,
+ metadata: nil,
+ trace: nil,
+ junit: 'junit.xml',
+ sast: 'gl-sast-report.json',
+ dependency_scanning: 'gl-dependency-scanning-report.json',
+ container_scanning: 'gl-container-scanning-report.json',
+ dast: 'gl-dast-report.json'
+ }.freeze
+
+ TYPE_AND_FORMAT_PAIRS = {
+ archive: :zip,
+ metadata: :gzip,
+ trace: :raw,
+ junit: :gzip,
+ sast: :gzip,
+ dependency_scanning: :gzip,
+ container_scanning: :gzip,
+ dast: :gzip
+ }.freeze
belongs_to :project
belongs_to :job, class_name: "Ci::Build", foreign_key: :job_id
@@ -27,8 +47,18 @@ module Ci
scope :with_files_stored_locally, -> { where(file_store: [nil, ::JobArtifactUploader::Store::LOCAL]) }
+ scope :with_file_types, -> (file_types) do
+ types = self.file_types.select { |file_type| file_types.include?(file_type) }.values
+
+ where(file_type: types)
+ end
+
scope :test_reports, -> do
- types = self.file_types.select { |file_type| TEST_REPORT_FILE_TYPES.include?(file_type) }.values
+ with_file_types(TEST_REPORT_FILE_TYPES)
+ end
+
+ scope :erasable, -> do
+ types = self.file_types.reject { |file_type| NON_ERASABLE_FILE_TYPES.include?(file_type) }.values
where(file_type: types)
end
@@ -39,7 +69,11 @@ module Ci
archive: 1,
metadata: 2,
trace: 3,
- junit: 4
+ junit: 4,
+ sast: 5, ## EE-specific
+ dependency_scanning: 6, ## EE-specific
+ container_scanning: 7, ## EE-specific
+ dast: 8 ## EE-specific
}
enum file_format: {
diff --git a/app/workers/expire_build_instance_artifacts_worker.rb b/app/workers/expire_build_instance_artifacts_worker.rb
index 4fcd1e5bd24..94426dcf921 100644
--- a/app/workers/expire_build_instance_artifacts_worker.rb
+++ b/app/workers/expire_build_instance_artifacts_worker.rb
@@ -13,7 +13,7 @@ class ExpireBuildInstanceArtifactsWorker
return unless build&.project && !build.project.pending_delete
Rails.logger.info "Removing artifacts for build #{build.id}..."
- build.erase_artifacts!
+ build.erase_erasable_artifacts!
end
# rubocop: enable CodeReuse/ActiveRecord
end
diff --git a/changelogs/unreleased/6717_extend_reports_for_security_products.yml b/changelogs/unreleased/6717_extend_reports_for_security_products.yml
new file mode 100644
index 00000000000..184c3212e54
--- /dev/null
+++ b/changelogs/unreleased/6717_extend_reports_for_security_products.yml
@@ -0,0 +1,5 @@
+---
+title: Extend reports feature to support Security Products
+merge_request: 21892
+author:
+type: added
diff --git a/lib/gitlab/ci/config/entry/reports.rb b/lib/gitlab/ci/config/entry/reports.rb
index 5963f3eb90c..7bc482a6f48 100644
--- a/lib/gitlab/ci/config/entry/reports.rb
+++ b/lib/gitlab/ci/config/entry/reports.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
module Gitlab
module Ci
class Config
@@ -9,7 +11,7 @@ module Gitlab
include Validatable
include Attributable
- ALLOWED_KEYS = %i[junit].freeze
+ ALLOWED_KEYS = %i[junit sast dependency_scanning container_scanning dast].freeze
attributes ALLOWED_KEYS
@@ -19,6 +21,10 @@ module Gitlab
with_options allow_nil: true do
validates :junit, array_of_strings_or_string: true
+ validates :sast, array_of_strings_or_string: true
+ validates :dependency_scanning, array_of_strings_or_string: true
+ validates :container_scanning, array_of_strings_or_string: true
+ validates :dast, array_of_strings_or_string: true
end
end
diff --git a/lib/gitlab/ci/parsers.rb b/lib/gitlab/ci/parsers.rb
deleted file mode 100644
index a4eccc08dfc..00000000000
--- a/lib/gitlab/ci/parsers.rb
+++ /dev/null
@@ -1,9 +0,0 @@
-module Gitlab
- module Ci
- module Parsers
- def self.fabricate!(file_type)
- "Gitlab::Ci::Parsers::#{file_type.classify}".constantize.new
- end
- end
- end
-end
diff --git a/lib/gitlab/ci/parsers/junit.rb b/lib/gitlab/ci/parsers/junit.rb
deleted file mode 100644
index d1c136f2009..00000000000
--- a/lib/gitlab/ci/parsers/junit.rb
+++ /dev/null
@@ -1,66 +0,0 @@
-module Gitlab
- module Ci
- module Parsers
- class Junit
- JunitParserError = Class.new(StandardError)
-
- def parse!(xml_data, test_suite)
- root = Hash.from_xml(xml_data)
-
- all_cases(root) do |test_case|
- test_case = create_test_case(test_case)
- test_suite.add_test_case(test_case)
- end
- rescue REXML::ParseException => e
- raise JunitParserError, "XML parsing failed: #{e.message}"
- rescue => e
- raise JunitParserError, "JUnit parsing failed: #{e.message}"
- end
-
- private
-
- def all_cases(root, parent = nil, &blk)
- return unless root.present?
-
- [root].flatten.compact.map do |node|
- next unless node.is_a?(Hash)
-
- # we allow only one top-level 'testsuites'
- all_cases(node['testsuites'], root, &blk) unless parent
-
- # we require at least one level of testsuites or testsuite
- each_case(node['testcase'], &blk) if parent
-
- # we allow multiple nested 'testsuite' (eg. PHPUnit)
- all_cases(node['testsuite'], root, &blk)
- end
- end
-
- def each_case(testcase, &blk)
- return unless testcase.present?
-
- [testcase].flatten.compact.map(&blk)
- end
-
- def create_test_case(data)
- if data['failure']
- status = ::Gitlab::Ci::Reports::TestCase::STATUS_FAILED
- system_output = data['failure']
- else
- status = ::Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS
- system_output = nil
- end
-
- ::Gitlab::Ci::Reports::TestCase.new(
- classname: data['classname'],
- name: data['name'],
- file: data['file'],
- execution_time: data['time'],
- status: status,
- system_output: system_output
- )
- end
- end
- end
- end
-end
diff --git a/lib/gitlab/ci/parsers/test.rb b/lib/gitlab/ci/parsers/test.rb
new file mode 100644
index 00000000000..c6bc9662b07
--- /dev/null
+++ b/lib/gitlab/ci/parsers/test.rb
@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module Ci
+ module Parsers
+ module Test
+ ParserNotFoundError = Class.new(StandardError)
+
+ PARSERS = {
+ junit: ::Gitlab::Ci::Parsers::Test::Junit
+ }.freeze
+
+ def self.fabricate!(file_type)
+ PARSERS.fetch(file_type.to_sym).new
+ rescue KeyError
+ raise ParserNotFoundError, "Cannot find any parser matching file type '#{file_type}'"
+ end
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/ci/parsers/test/junit.rb b/lib/gitlab/ci/parsers/test/junit.rb
new file mode 100644
index 00000000000..5d7d9a751d8
--- /dev/null
+++ b/lib/gitlab/ci/parsers/test/junit.rb
@@ -0,0 +1,70 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module Ci
+ module Parsers
+ module Test
+ class Junit
+ JunitParserError = Class.new(StandardError)
+
+ def parse!(xml_data, test_suite)
+ root = Hash.from_xml(xml_data)
+
+ all_cases(root) do |test_case|
+ test_case = create_test_case(test_case)
+ test_suite.add_test_case(test_case)
+ end
+ rescue REXML::ParseException => e
+ raise JunitParserError, "XML parsing failed: #{e.message}"
+ rescue => e
+ raise JunitParserError, "JUnit parsing failed: #{e.message}"
+ end
+
+ private
+
+ def all_cases(root, parent = nil, &blk)
+ return unless root.present?
+
+ [root].flatten.compact.map do |node|
+ next unless node.is_a?(Hash)
+
+ # we allow only one top-level 'testsuites'
+ all_cases(node['testsuites'], root, &blk) unless parent
+
+ # we require at least one level of testsuites or testsuite
+ each_case(node['testcase'], &blk) if parent
+
+ # we allow multiple nested 'testsuite' (eg. PHPUnit)
+ all_cases(node['testsuite'], root, &blk)
+ end
+ end
+
+ def each_case(testcase, &blk)
+ return unless testcase.present?
+
+ [testcase].flatten.compact.map(&blk)
+ end
+
+ def create_test_case(data)
+ if data['failure']
+ status = ::Gitlab::Ci::Reports::TestCase::STATUS_FAILED
+ system_output = data['failure']
+ else
+ status = ::Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS
+ system_output = nil
+ end
+
+ ::Gitlab::Ci::Reports::TestCase.new(
+ classname: data['classname'],
+ name: data['name'],
+ file: data['file'],
+ execution_time: data['time'],
+ status: status,
+ system_output: system_output
+ )
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/spec/factories/ci/job_artifacts.rb b/spec/factories/ci/job_artifacts.rb
index f028803ca74..ee79f0ae5fd 100644
--- a/spec/factories/ci/job_artifacts.rb
+++ b/spec/factories/ci/job_artifacts.rb
@@ -14,6 +14,33 @@ FactoryBot.define do
artifact.project ||= artifact.job.project
end
+ trait :raw do
+ file_format :raw
+
+ after(:build) do |artifact, _|
+ artifact.file = fixture_file_upload(
+ Rails.root.join('spec/fixtures/trace/sample_trace'), 'text/plain')
+ end
+ end
+
+ trait :zip do
+ file_format :zip
+
+ after(:build) do |artifact, _|
+ artifact.file = fixture_file_upload(
+ Rails.root.join('spec/fixtures/ci_build_artifacts.zip'), 'application/zip')
+ end
+ end
+
+ trait :gzip do
+ file_format :gzip
+
+ after(:build) do |artifact, _|
+ artifact.file = fixture_file_upload(
+ Rails.root.join('spec/fixtures/ci_build_artifacts_metadata.gz'), 'application/x-gzip')
+ end
+ end
+
trait :archive do
file_type :archive
file_format :zip
diff --git a/spec/lib/gitlab/ci/config/entry/reports_spec.rb b/spec/lib/gitlab/ci/config/entry/reports_spec.rb
index b3a3a6bee1d..e5fc36b5d7a 100644
--- a/spec/lib/gitlab/ci/config/entry/reports_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/reports_spec.rb
@@ -3,27 +3,53 @@ require 'spec_helper'
describe Gitlab::Ci::Config::Entry::Reports do
let(:entry) { described_class.new(config) }
+ describe 'validates ALLOWED_KEYS' do
+ let(:artifact_file_types) { Ci::JobArtifact.file_types }
+
+ described_class::ALLOWED_KEYS.each do |keyword, _|
+ it "expects #{keyword} to be an artifact file_type" do
+ expect(artifact_file_types).to include(keyword)
+ end
+ end
+ end
+
describe 'validation' do
context 'when entry config value is correct' do
- let(:config) { { junit: %w[junit.xml] } }
+ using RSpec::Parameterized::TableSyntax
- describe '#value' do
- it 'returns artifacs configuration' do
- expect(entry.value).to eq config
+ shared_examples 'a valid entry' do |keyword, file|
+ describe '#value' do
+ it 'returns artifacs configuration' do
+ expect(entry.value).to eq({ "#{keyword}": [file] } )
+ end
end
- end
- describe '#valid?' do
- it 'is valid' do
- expect(entry).to be_valid
+ describe '#valid?' do
+ it 'is valid' do
+ expect(entry).to be_valid
+ end
end
end
- context 'when value is not array' do
- let(:config) { { junit: 'junit.xml' } }
+ where(:keyword, :file) do
+ :junit | 'junit.xml'
+ :sast | 'gl-sast-report.json'
+ :dependency_scanning | 'gl-dependency-scanning-report.json'
+ :container_scanning | 'gl-container-scanning-report.json'
+ :dast | 'gl-dast-report.json'
+ end
+
+ with_them do
+ context 'when value is an array' do
+ let(:config) { { "#{keyword}": [file] } }
- it 'converts to array' do
- expect(entry.value).to eq({ junit: ['junit.xml'] } )
+ it_behaves_like 'a valid entry', params[:keyword], params[:file]
+ end
+
+ context 'when value is not array' do
+ let(:config) { { "#{keyword}": file } }
+
+ it_behaves_like 'a valid entry', params[:keyword], params[:file]
end
end
end
@@ -31,11 +57,13 @@ describe Gitlab::Ci::Config::Entry::Reports do
context 'when entry value is not correct' do
describe '#errors' do
context 'when value of attribute is invalid' do
- let(:config) { { junit: 10 } }
+ where(key: described_class::ALLOWED_KEYS) do
+ let(:config) { { "#{key}": 10 } }
- it 'reports error' do
- expect(entry.errors)
- .to include 'reports junit should be an array of strings or a string'
+ it 'reports error' do
+ expect(entry.errors)
+ .to include "reports #{key} should be an array of strings or a string"
+ end
end
end
diff --git a/spec/lib/gitlab/ci/parsers/junit_spec.rb b/spec/lib/gitlab/ci/parsers/test/junit_spec.rb
index 47497f06229..a49402c7398 100644
--- a/spec/lib/gitlab/ci/parsers/junit_spec.rb
+++ b/spec/lib/gitlab/ci/parsers/test/junit_spec.rb
@@ -1,6 +1,6 @@
require 'fast_spec_helper'
-describe Gitlab::Ci::Parsers::Junit do
+describe Gitlab::Ci::Parsers::Test::Junit do
describe '#parse!' do
subject { described_class.new.parse!(junit, test_suite) }
diff --git a/spec/lib/gitlab/ci/parsers_spec.rb b/spec/lib/gitlab/ci/parsers/test_spec.rb
index 2fa83c4abae..0b85b432677 100644
--- a/spec/lib/gitlab/ci/parsers_spec.rb
+++ b/spec/lib/gitlab/ci/parsers/test_spec.rb
@@ -1,6 +1,6 @@
require 'spec_helper'
-describe Gitlab::Ci::Parsers do
+describe Gitlab::Ci::Parsers::Test do
describe '.fabricate!' do
subject { described_class.fabricate!(file_type) }
@@ -16,7 +16,7 @@ describe Gitlab::Ci::Parsers do
let(:file_type) { 'undefined' }
it 'raises an error' do
- expect { subject }.to raise_error(NameError)
+ expect { subject }.to raise_error(Gitlab::Ci::Parsers::Test::ParserNotFoundError)
end
end
end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index dbebda20ce0..e82d93d5935 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -176,9 +176,7 @@ describe Ci::Build do
it 'does not execute a query for selecting job artifact one by one' do
recorded = ActiveRecord::QueryRecorder.new do
subject.each do |build|
- Ci::JobArtifact::TEST_REPORT_FILE_TYPES.each do |file_type|
- build.public_send("job_artifacts_#{file_type}").file.exists?
- end
+ build.job_artifacts.map { |a| a.file.exists? }
end
end
@@ -550,44 +548,22 @@ describe Ci::Build do
end
end
- describe '#has_test_reports?' do
- subject { build.has_test_reports? }
+ describe '#has_job_artifacts?' do
+ subject { build.has_job_artifacts? }
- context 'when build has a test report' do
- let(:build) { create(:ci_build, :test_reports) }
+ context 'when build has a job artifact' do
+ let(:build) { create(:ci_build, :artifacts) }
it { is_expected.to be_truthy }
end
- context 'when build does not have test reports' do
- let(:build) { create(:ci_build, :artifacts) }
+ context 'when build does not have job artifacts' do
+ let(:build) { create(:ci_build, :legacy_artifacts) }
it { is_expected.to be_falsy }
end
end
- describe '#erase_test_reports!' do
- subject { build.erase_test_reports! }
-
- context 'when build has a test report' do
- let!(:build) { create(:ci_build, :test_reports) }
-
- it 'removes a test report' do
- subject
-
- expect(build.has_test_reports?).to be_falsy
- end
- end
-
- context 'when build does not have test reports' do
- let!(:build) { create(:ci_build, :artifacts) }
-
- it 'does not erase anything' do
- expect { subject }.not_to change { Ci::JobArtifact.count }
- end
- end
- end
-
describe '#has_old_trace?' do
subject { build.has_old_trace? }
@@ -850,8 +826,8 @@ describe Ci::Build do
expect(build.artifacts_metadata.exists?).to be_falsy
end
- it 'removes test reports' do
- expect(build.job_artifacts.test_reports.count).to eq(0)
+ it 'removes all job_artifacts' do
+ expect(build.job_artifacts.count).to eq(0)
end
it 'erases build trace in trace file' do
@@ -1022,6 +998,32 @@ describe Ci::Build do
end
end
+ describe '#erase_erasable_artifacts!' do
+ let!(:build) { create(:ci_build, :success) }
+
+ subject { build.erase_erasable_artifacts! }
+
+ before do
+ Ci::JobArtifact.file_types.keys.each do |file_type|
+ create(:ci_job_artifact, job: build, file_type: file_type, file_format: Ci::JobArtifact::TYPE_AND_FORMAT_PAIRS[file_type.to_sym])
+ end
+ end
+
+ it "erases erasable artifacts" do
+ subject
+
+ expect(build.job_artifacts.erasable).to be_empty
+ end
+
+ it "keeps non erasable artifacts" do
+ subject
+
+ Ci::JobArtifact::NON_ERASABLE_FILE_TYPES.each do |file_type|
+ expect(build.send("job_artifacts_#{file_type}")).not_to be_nil
+ end
+ end
+ end
+
describe '#first_pending' do
let!(:first) { create(:ci_build, pipeline: pipeline, status: 'pending', created_at: Date.yesterday) }
let!(:second) { create(:ci_build, pipeline: pipeline, status: 'pending') }
@@ -2844,16 +2846,10 @@ describe Ci::Build do
end
it 'raises an error' do
- expect { subject }.to raise_error(Gitlab::Ci::Parsers::Junit::JunitParserError)
+ expect { subject }.to raise_error(Gitlab::Ci::Parsers::Test::Junit::JunitParserError)
end
end
end
-
- context 'when build does not have test reports' do
- it 'raises an error' do
- expect { subject }.to raise_error(NoMethodError)
- end
- end
end
describe '#artifacts_metadata_entry' do
diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb
index 1bf338f4c70..2f449d617be 100644
--- a/spec/models/ci/job_artifact_spec.rb
+++ b/spec/models/ci/job_artifact_spec.rb
@@ -31,6 +31,22 @@ describe Ci::JobArtifact do
end
end
+ describe '.erasable' do
+ subject { described_class.erasable }
+
+ context 'when there is am erasable artifact' do
+ let!(:artifact) { create(:ci_job_artifact, :junit) }
+
+ it { is_expected.to eq([artifact]) }
+ end
+
+ context 'when there are no erasable artifacts' do
+ let!(:artifact) { create(:ci_job_artifact, :trace) }
+
+ it { is_expected.to be_empty }
+ end
+ end
+
describe 'callbacks' do
subject { create(:ci_job_artifact, :archive) }
@@ -106,34 +122,46 @@ describe Ci::JobArtifact do
describe 'validates file format' do
subject { artifact }
- context 'when archive type with zip format' do
- let(:artifact) { build(:ci_job_artifact, :archive, file_format: :zip) }
+ described_class::TYPE_AND_FORMAT_PAIRS.except(:trace).each do |file_type, file_format|
+ context "when #{file_type} type with #{file_format} format" do
+ let(:artifact) { build(:ci_job_artifact, file_type: file_type, file_format: file_format) }
- it { is_expected.to be_valid }
- end
+ it { is_expected.to be_valid }
+ end
- context 'when archive type with gzip format' do
- let(:artifact) { build(:ci_job_artifact, :archive, file_format: :gzip) }
+ context "when #{file_type} type without format specification" do
+ let(:artifact) { build(:ci_job_artifact, file_type: file_type, file_format: nil) }
- it { is_expected.not_to be_valid }
- end
+ it { is_expected.not_to be_valid }
+ end
- context 'when archive type without format specification' do
- let(:artifact) { build(:ci_job_artifact, :archive, file_format: nil) }
+ context "when #{file_type} type with other formats" do
+ described_class.file_formats.except(file_format).values.each do |other_format|
+ let(:artifact) { build(:ci_job_artifact, file_type: file_type, file_format: other_format) }
- it { is_expected.not_to be_valid }
+ it { is_expected.not_to be_valid }
+ end
+ end
end
+ end
- context 'when junit type with zip format' do
- let(:artifact) { build(:ci_job_artifact, :junit, file_format: :zip) }
+ describe 'validates DEFAULT_FILE_NAMES' do
+ subject { described_class::DEFAULT_FILE_NAMES }
- it { is_expected.not_to be_valid }
+ described_class.file_types.each do |file_type, _|
+ it "expects #{file_type} to be included" do
+ is_expected.to include(file_type.to_sym)
+ end
end
+ end
- context 'when junit type with gzip format' do
- let(:artifact) { build(:ci_job_artifact, :junit, file_format: :gzip) }
+ describe 'validates TYPE_AND_FORMAT_PAIRS' do
+ subject { described_class::TYPE_AND_FORMAT_PAIRS }
- it { is_expected.to be_valid }
+ described_class.file_types.each do |file_type, _|
+ it "expects #{file_type} to be included" do
+ expect(described_class.file_formats).to include(subject[file_type.to_sym])
+ end
end
end
diff --git a/spec/presenters/ci/build_runner_presenter_spec.rb b/spec/presenters/ci/build_runner_presenter_spec.rb
index e7019b990dd..a42d1f3d399 100644
--- a/spec/presenters/ci/build_runner_presenter_spec.rb
+++ b/spec/presenters/ci/build_runner_presenter_spec.rb
@@ -3,7 +3,6 @@ require 'spec_helper'
describe Ci::BuildRunnerPresenter do
let(:presenter) { described_class.new(build) }
let(:archive) { { paths: ['sample.txt'] } }
- let(:junit) { { junit: ['junit.xml'] } }
let(:archive_expectation) do
{
@@ -14,16 +13,6 @@ describe Ci::BuildRunnerPresenter do
}
end
- let(:junit_expectation) do
- {
- name: 'junit.xml',
- artifact_type: :junit,
- artifact_format: :gzip,
- paths: ['junit.xml'],
- when: 'always'
- }
- end
-
describe '#artifacts' do
context "when option contains archive-type artifacts" do
let(:build) { create(:ci_build, options: { artifacts: archive } ) }
@@ -49,20 +38,44 @@ describe Ci::BuildRunnerPresenter do
end
end
- context "when option has 'junit' keyword" do
- let(:build) { create(:ci_build, options: { artifacts: { reports: junit } } ) }
+ context "with reports" do
+ Ci::JobArtifact::DEFAULT_FILE_NAMES.each do |file_type, filename|
+ let(:report) { { "#{file_type}": [filename] } }
+ let(:build) { create(:ci_build, options: { artifacts: { reports: report } } ) }
+
+ let(:report_expectation) do
+ {
+ name: filename,
+ artifact_type: :"#{file_type}",
+ artifact_format: :gzip,
+ paths: [filename],
+ when: 'always'
+ }
+ end
- it 'presents correct hash' do
- expect(presenter.artifacts.first).to include(junit_expectation)
+ it 'presents correct hash' do
+ expect(presenter.artifacts.first).to include(report_expectation)
+ end
end
end
context "when option has both archive and reports specification" do
- let(:build) { create(:ci_build, options: { script: 'echo', artifacts: { **archive, reports: junit } } ) }
+ let(:report) { { junit: ['junit.xml'] } }
+ let(:build) { create(:ci_build, options: { script: 'echo', artifacts: { **archive, reports: report } } ) }
+
+ let(:report_expectation) do
+ {
+ name: 'junit.xml',
+ artifact_type: :junit,
+ artifact_format: :gzip,
+ paths: ['junit.xml'],
+ when: 'always'
+ }
+ end
it 'presents correct hash' do
expect(presenter.artifacts.first).to include(archive_expectation)
- expect(presenter.artifacts.second).to include(junit_expectation)
+ expect(presenter.artifacts.second).to include(report_expectation)
end
context "when archive specifies 'expire_in' keyword" do
@@ -70,7 +83,7 @@ describe Ci::BuildRunnerPresenter do
it 'inherits expire_in from archive' do
expect(presenter.artifacts.first).to include({ **archive_expectation, expire_in: '3 mins 4 sec' })
- expect(presenter.artifacts.second).to include({ **junit_expectation, expire_in: '3 mins 4 sec' })
+ expect(presenter.artifacts.second).to include({ **report_expectation, expire_in: '3 mins 4 sec' })
end
end
end
diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb
index 6adbbb40489..8770365c893 100644
--- a/spec/requests/api/jobs_spec.rb
+++ b/spec/requests/api/jobs_spec.rb
@@ -721,7 +721,7 @@ describe API::Jobs do
expect(job.trace.exist?).to be_falsy
expect(job.artifacts_file.exists?).to be_falsy
expect(job.artifacts_metadata.exists?).to be_falsy
- expect(job.has_test_reports?).to be_falsy
+ expect(job.has_job_artifacts?).to be_falsy
end
it 'updates job' do
diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb
index 18d52082399..951c0b16a68 100644
--- a/spec/services/ci/retry_build_service_spec.rb
+++ b/spec/services/ci/retry_build_service_spec.rb
@@ -24,7 +24,9 @@ describe Ci::RetryBuildService do
artifacts_file artifacts_metadata artifacts_size created_at
updated_at started_at finished_at queued_at erased_by
erased_at auto_canceled_by job_artifacts job_artifacts_archive
- job_artifacts_metadata job_artifacts_trace job_artifacts_junit].freeze
+ job_artifacts_metadata job_artifacts_trace job_artifacts_junit
+ job_artifacts_sast job_artifacts_dependency_scanning
+ job_artifacts_container_scanning job_artifacts_dast].freeze
IGNORE_ACCESSORS =
%i[type lock_version target_url base_tags trace_sections
@@ -38,9 +40,8 @@ describe Ci::RetryBuildService do
let(:another_pipeline) { create(:ci_empty_pipeline, project: project) }
let(:build) do
- create(:ci_build, :failed, :artifacts, :test_reports, :expired, :erased,
- :queued, :coverage, :tags, :allowed_to_fail, :on_tag,
- :triggered, :trace_artifact, :teardown_environment,
+ create(:ci_build, :failed, :expired, :erased, :queued, :coverage, :tags,
+ :allowed_to_fail, :on_tag, :triggered, :teardown_environment,
description: 'my-job', stage: 'test', stage_id: stage.id,
pipeline: pipeline, auto_canceled_by: another_pipeline)
end
@@ -50,6 +51,15 @@ describe Ci::RetryBuildService do
# can reset one of the fields when assigning another. We plan to deprecate
# and remove legacy `stage` column in the future.
build.update(stage: 'test', stage_id: stage.id)
+
+ # Make sure we have one instance for every possible job_artifact_X
+ # associations to check they are correctly rejected on build duplication.
+ Ci::JobArtifact::TYPE_AND_FORMAT_PAIRS.each do |file_type, file_format|
+ create(:ci_job_artifact, file_format,
+ file_type: file_type, job: build, expire_at: build.artifacts_expire_at)
+ end
+
+ build.reload
end
describe 'clone accessors' do