Commit 166b9307 authored by Igor Drozdov's avatar Igor Drozdov

View raw file of any zip artifacts

Currently we view raw content of any file in a zip archive
What about allowing to view the raw representation of any zip artifact?
parent 41f6db67
...@@ -10,7 +10,7 @@ class Projects::ArtifactsController < Projects::ApplicationController ...@@ -10,7 +10,7 @@ class Projects::ArtifactsController < Projects::ApplicationController
before_action :authorize_update_build!, only: [:keep] before_action :authorize_update_build!, only: [:keep]
before_action :authorize_destroy_artifacts!, only: [:destroy] before_action :authorize_destroy_artifacts!, only: [:destroy]
before_action :extract_ref_name_and_path before_action :extract_ref_name_and_path
before_action :validate_artifacts!, except: [:index, :download, :destroy] before_action :validate_artifacts!, except: [:index, :download, :raw, :destroy]
before_action :entry, only: [:file] before_action :entry, only: [:file]
MAX_PER_PAGE = 20 MAX_PER_PAGE = 20
...@@ -73,9 +73,11 @@ class Projects::ArtifactsController < Projects::ApplicationController ...@@ -73,9 +73,11 @@ class Projects::ArtifactsController < Projects::ApplicationController
end end
def raw def raw
return render_404 unless zip_artifact?
path = Gitlab::Ci::Build::Artifacts::Path.new(params[:path]) path = Gitlab::Ci::Build::Artifacts::Path.new(params[:path])
send_artifacts_entry(build, path) send_artifacts_entry(artifacts_file, path)
end end
def keep def keep
...@@ -138,6 +140,13 @@ class Projects::ArtifactsController < Projects::ApplicationController ...@@ -138,6 +140,13 @@ class Projects::ArtifactsController < Projects::ApplicationController
@artifacts_file ||= build&.artifacts_file_for_type(params[:file_type] || :archive) @artifacts_file ||= build&.artifacts_file_for_type(params[:file_type] || :archive)
end end
def zip_artifact?
types = HashWithIndifferentAccess.new(Ci::JobArtifact::TYPE_AND_FORMAT_PAIRS)
file_type = params[:file_type] || :archive
types[file_type] == :zip
end
def entry def entry
@entry = build.artifacts_metadata_entry(params[:path]) @entry = build.artifacts_metadata_entry(params[:path])
......
...@@ -36,8 +36,8 @@ module WorkhorseHelper ...@@ -36,8 +36,8 @@ module WorkhorseHelper
end end
# Send an entry from artifacts through Workhorse # Send an entry from artifacts through Workhorse
def send_artifacts_entry(build, entry) def send_artifacts_entry(file, entry)
headers.store(*Gitlab::Workhorse.send_artifacts_entry(build, entry)) headers.store(*Gitlab::Workhorse.send_artifacts_entry(file, entry))
head :ok head :ok
end end
......
...@@ -50,10 +50,10 @@ module Ci ...@@ -50,10 +50,10 @@ module Ci
metrics: :gzip, metrics: :gzip,
metrics_referee: :gzip, metrics_referee: :gzip,
network_referee: :gzip, network_referee: :gzip,
lsif: :gzip,
dotenv: :gzip, dotenv: :gzip,
cobertura: :gzip, cobertura: :gzip,
cluster_applications: :gzip, cluster_applications: :gzip,
lsif: :zip,
# All these file formats use `raw` as we need to store them uncompressed # All these file formats use `raw` as we need to store them uncompressed
# for Frontend to fetch the files and do analysis # for Frontend to fetch the files and do analysis
......
---
title: View raw file of any zip artifacts
merge_request: 31912
author:
type: added
...@@ -609,8 +609,8 @@ module API ...@@ -609,8 +609,8 @@ module API
header(*Gitlab::Workhorse.send_git_archive(repository, **kwargs)) header(*Gitlab::Workhorse.send_git_archive(repository, **kwargs))
end end
def send_artifacts_entry(build, entry) def send_artifacts_entry(file, entry)
header(*Gitlab::Workhorse.send_artifacts_entry(build, entry)) header(*Gitlab::Workhorse.send_artifacts_entry(file, entry))
end end
# The Grape Error Middleware only has access to `env` but not `params` nor # The Grape Error Middleware only has access to `env` but not `params` nor
......
...@@ -54,7 +54,7 @@ module API ...@@ -54,7 +54,7 @@ module API
bad_request! unless path.valid? bad_request! unless path.valid?
send_artifacts_entry(build, path) send_artifacts_entry(build.artifacts_file, path)
end end
desc 'Download the artifacts archive from a job' do desc 'Download the artifacts archive from a job' do
...@@ -90,7 +90,7 @@ module API ...@@ -90,7 +90,7 @@ module API
bad_request! unless path.valid? bad_request! unless path.valid?
send_artifacts_entry(build, path) send_artifacts_entry(build.artifacts_file, path)
end end
desc 'Keep the artifacts to prevent them from being deleted' do desc 'Keep the artifacts to prevent them from being deleted' do
......
...@@ -130,8 +130,7 @@ module Gitlab ...@@ -130,8 +130,7 @@ module Gitlab
] ]
end end
def send_artifacts_entry(build, entry) def send_artifacts_entry(file, entry)
file = build.artifacts_file
archive = file.file_storage? ? file.path : file.url archive = file.file_storage? ? file.path : file.url
params = { params = {
......
...@@ -308,10 +308,13 @@ describe Projects::ArtifactsController do ...@@ -308,10 +308,13 @@ describe Projects::ArtifactsController do
end end
describe 'GET raw' do describe 'GET raw' do
subject { get(:raw, params: { namespace_id: project.namespace, project_id: project, job_id: job, path: path }) } let(:query_params) { { namespace_id: project.namespace, project_id: project, job_id: job, path: path } }
subject { get(:raw, params: query_params) }
context 'when the file exists' do context 'when the file exists' do
let(:path) { 'ci_artifacts.txt' } let(:path) { 'ci_artifacts.txt' }
let(:archive_matcher) { /build_artifacts.zip(\?[^?]+)?$/ }
shared_examples 'a valid file' do shared_examples 'a valid file' do
it 'serves the file using workhorse' do it 'serves the file using workhorse' do
...@@ -323,8 +326,8 @@ describe Projects::ArtifactsController do ...@@ -323,8 +326,8 @@ describe Projects::ArtifactsController do
expect(params.keys).to eq(%w(Archive Entry)) expect(params.keys).to eq(%w(Archive Entry))
expect(params['Archive']).to start_with(archive_path) expect(params['Archive']).to start_with(archive_path)
# On object storage, the URL can end with a query string # On object storage, the URL can end with a query string
expect(params['Archive']).to match(/build_artifacts.zip(\?[^?]+)?$/) expect(params['Archive']).to match(archive_matcher)
expect(params['Entry']).to eq(Base64.encode64('ci_artifacts.txt')) expect(params['Entry']).to eq(Base64.encode64(path))
end end
def send_data def send_data
...@@ -359,6 +362,37 @@ describe Projects::ArtifactsController do ...@@ -359,6 +362,37 @@ describe Projects::ArtifactsController do
let(:archive_path) { 'https://' } let(:archive_path) { 'https://' }
end end
end end
context 'fetching an artifact of different type' do
before do
job.job_artifacts.each(&:destroy)
end
context 'when the artifact is zip' do
let!(:artifact) { create(:ci_job_artifact, :lsif, job: job, file_path: Rails.root.join("spec/fixtures/#{file_name}")) }
let(:path) { 'lsif/main.go.json' }
let(:file_name) { 'lsif.json.zip' }
let(:archive_matcher) { file_name }
let(:query_params) { super().merge(file_type: :lsif, path: path) }
it_behaves_like 'a valid file' do
let(:store) { ObjectStorage::Store::LOCAL }
let(:archive_path) { JobArtifactUploader.root }
end
end
context 'when the artifact is not zip' do
let(:query_params) { super().merge(file_type: :junit, path: '') }
it 'responds with not found' do
create(:ci_job_artifact, :junit, job: job)
subject
expect(response).to have_gitlab_http_status(:not_found)
end
end
end
end end
end end
......
...@@ -231,11 +231,14 @@ FactoryBot.define do ...@@ -231,11 +231,14 @@ FactoryBot.define do
trait :lsif do trait :lsif do
file_type { :lsif } file_type { :lsif }
file_format { :gzip } file_format { :zip }
transient do
file_path { Rails.root.join('spec/fixtures/lsif.json.gz') }
end
after(:build) do |artifact, evaluator| after(:build) do |artifact, evaluator|
artifact.file = fixture_file_upload( artifact.file = fixture_file_upload(evaluator.file_path, 'application/x-gzip')
Rails.root.join('spec/fixtures/lsif.json.gz'), 'application/x-gzip')
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