Checking filr_format and file_type paring

3 files changed, 77 insertions, 9 deletions

diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb
index 6fc2d9e8282..a736e2b0e22 100644
--- a/app/models/ci/job_artifact.rb
+++ b/app/models/ci/job_artifact.rb
@@ -10,6 +10,7 @@ module Ci
     mount_uploader :file, JobArtifactUploader
 
     validates :file_format, presence: true, unless: :ignore_schema, on: :create
+    validate :valid_file_format?, unless: :ignore_schema, on: :create
     before_save :set_size, if: :file_changed?
     after_save :update_project_statistics_after_save, if: :size_changed?
     after_destroy :update_project_statistics_after_destroy, unless: :project_destroyed?
@@ -36,6 +37,7 @@ module Ci
     GENERAL_ARCHIVE_FILE_TYPE = 'archive'.freeze
     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
 
     enum file_format: {
       raw: 1,
@@ -43,6 +45,12 @@ module Ci
       gzip: 3
     }
 
+    def valid_file_format?
+      unless TYPE_AND_FORMAT_PAIRS[self.file_type&.to_sym] == self.file_format&.to_sym
+        errors.add(:file_format, 'Invalid file format with specified file type')
+      end
+    end
+
     def ignore_schema
       ActiveRecord::Migrator.current_version <= ::Gitlab::Ci::Trace::ARCHIVE_LEGACY_TRACES_MIGRATION_VERSION
     end
diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb
index 594211c288f..4474df73434 100644
--- a/spec/models/ci/job_artifact_spec.rb
+++ b/spec/models/ci/job_artifact_spec.rb
@@ -90,25 +90,35 @@ describe Ci::JobArtifact do
   describe 'validates file format' do
     subject { artifact.valid? }
 
-    let(:artifact) { build(:ci_job_artifact, file_format: file_format) }
-
-    context 'when file format is present' do
-      let(:file_format) { :raw }
+    context 'when archive type with zip format' do
+      let(:artifact) { build(:ci_job_artifact, :archive, file_format: :zip) }
 
       it { is_expected.to be_truthy }
     end
 
-    context 'when file format is not defined' do
-      let(:file_format) { :new_format }
+    context 'when archive type with gzip format' do
+      let(:artifact) { build(:ci_job_artifact, :archive, file_format: :gzip) }
+
+      it { is_expected.to be_falsy }
+    end
+
+    context 'when archive type without format specification' do
+      let(:artifact) { build(:ci_job_artifact, :archive, file_format: nil) }
 
-      it { expect { subject }.to raise_error(ArgumentError) }
+      it { is_expected.to be_falsy }
     end
 
-    context 'when file format is not present' do
-      let(:file_format) { nil }
+    context 'when junit type with zip format' do
+      let(:artifact) { build(:ci_job_artifact, :junit, file_format: :zip) }
 
       it { is_expected.to be_falsy }
     end
+
+    context 'when junit type with gzip format' do
+      let(:artifact) { build(:ci_job_artifact, :junit, file_format: :gzip) }
+
+      it { is_expected.to be_truthy }
+    end
   end
 
   describe '#file' do
diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb
index 77efb877662..0f41e46cdd5 100644
--- a/spec/requests/api/runner_spec.rb
+++ b/spec/requests/api/runner_spec.rb
@@ -1422,6 +1422,56 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
               end
             end
           end
+
+          context 'when artifact_type is archive' do
+            context 'when artifact_format is zip' do
+              let(:params) { { artifact_type: :archive, artifact_format: :zip } }
+
+              it 'stores junit test report' do
+                upload_artifacts(file_upload, headers_with_token, params)
+
+                expect(response).to have_gitlab_http_status(201)
+                expect(job.reload.job_artifacts_archive).not_to be_nil
+              end
+            end
+
+            context 'when artifact_format is gzip' do
+              let(:params) { { artifact_type: :archive, artifact_format: :gzip } }
+
+              it 'returns an error' do
+                upload_artifacts(file_upload, headers_with_token, params)
+
+                expect(response).to have_gitlab_http_status(400)
+                expect(job.reload.job_artifacts_archive).to be_nil
+              end
+            end
+          end
+
+          context 'when artifact_type is junit' do
+            context 'when artifact_format is gzip' do
+              let(:file_upload) { fixture_file_upload('spec/fixtures/junit.xml.gz') }
+              let(:params) { { artifact_type: :junit, artifact_format: :gzip } }
+
+              it 'stores junit test report' do
+                upload_artifacts(file_upload, headers_with_token, params)
+
+                expect(response).to have_gitlab_http_status(201)
+                expect(job.reload.job_artifacts_junit).not_to be_nil
+              end
+            end
+
+            context 'when artifact_format is raw' do
+              let(:file_upload) { fixture_file_upload('spec/fixtures/junit.xml.gz') }
+              let(:params) { { artifact_type: :junit, artifact_format: :raw } }
+
+              it 'returns an error' do
+                upload_artifacts(file_upload, headers_with_token, params)
+
+                expect(response).to have_gitlab_http_status(400)
+                expect(job.reload.job_artifacts_junit).to be_nil
+              end
+            end
+          end
         end
 
         context 'when artifacts are being stored outside of tmp path' do