Commit 418ad84e authored by Sean McGivern's avatar Sean McGivern

Merge branch 'eb-custom-max-artifacts-size' into 'master'

Update artifact size authorization

See merge request gitlab-org/gitlab!17769
parents 3079aa62 28fa95fe
...@@ -316,6 +316,12 @@ class Namespace < ApplicationRecord ...@@ -316,6 +316,12 @@ class Namespace < ApplicationRecord
Pages::VirtualDomain.new(all_projects_with_pages, trim_prefix: full_path) Pages::VirtualDomain.new(all_projects_with_pages, trim_prefix: full_path)
end end
def closest_setting(name)
self_and_ancestors(hierarchy_order: :asc)
.find { |n| !n.read_attribute(name).nil? }
.try(name)
end
private private
def all_projects_with_pages def all_projects_with_pages
......
...@@ -2250,8 +2250,23 @@ class Project < ApplicationRecord ...@@ -2250,8 +2250,23 @@ class Project < ApplicationRecord
Pages::LookupPath.new(self, trim_prefix: trim_prefix, domain: domain) Pages::LookupPath.new(self, trim_prefix: trim_prefix, domain: domain)
end end
def closest_setting(name)
setting = read_attribute(name)
setting = closest_namespace_setting(name) if setting.nil?
setting = app_settings_for(name) if setting.nil?
setting
end
private private
def closest_namespace_setting(name)
namespace.closest_setting(name)
end
def app_settings_for(name)
Gitlab::CurrentSettings.send(name) # rubocop:disable GitlabSecurity/PublicSend
end
def merge_requests_allowing_collaboration(source_branch = nil) def merge_requests_allowing_collaboration(source_branch = nil)
relation = source_of_merge_requests.opened.where(allow_collaboration: true) relation = source_of_merge_requests.opened.where(allow_collaboration: true)
relation = relation.where(source_branch: source_branch) if source_branch relation = relation.where(source_branch: source_branch) if source_branch
......
...@@ -350,7 +350,7 @@ module API ...@@ -350,7 +350,7 @@ module API
render_api_error!(message || '409 Conflict', 409) render_api_error!(message || '409 Conflict', 409)
end end
def file_to_large! def file_too_large!
render_api_error!('413 Request Entity Too Large', 413) render_api_error!('413 Request Entity Too Large', 413)
end end
......
...@@ -59,8 +59,9 @@ module API ...@@ -59,8 +59,9 @@ module API
token && job.valid_token?(token) token && job.valid_token?(token)
end end
def max_artifacts_size def max_artifacts_size(job)
Gitlab::CurrentSettings.max_artifacts_size.megabytes.to_i max_size = job.project.closest_setting(:max_artifacts_size)
max_size.megabytes.to_i
end end
def job_forbidden!(job, reason) def job_forbidden!(job, reason)
......
...@@ -221,14 +221,16 @@ module API ...@@ -221,14 +221,16 @@ module API
job = authenticate_job! job = authenticate_job!
forbidden!('Job is not running') unless job.running? forbidden!('Job is not running') unless job.running?
max_size = max_artifacts_size(job)
if params[:filesize] if params[:filesize]
file_size = params[:filesize].to_i file_size = params[:filesize].to_i
file_to_large! unless file_size < max_artifacts_size file_too_large! unless file_size < max_size
end end
status 200 status 200
content_type Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE content_type Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE
JobArtifactUploader.workhorse_authorize(has_length: false, maximum_size: max_artifacts_size) JobArtifactUploader.workhorse_authorize(has_length: false, maximum_size: max_size)
end end
desc 'Upload artifacts for job' do desc 'Upload artifacts for job' do
...@@ -268,7 +270,7 @@ module API ...@@ -268,7 +270,7 @@ module API
metadata = UploadedFile.from_params(params, :metadata, JobArtifactUploader.workhorse_local_upload_path) metadata = UploadedFile.from_params(params, :metadata, JobArtifactUploader.workhorse_local_upload_path)
bad_request!('Missing artifacts file!') unless artifacts bad_request!('Missing artifacts file!') unless artifacts
file_to_large! unless artifacts.size < max_artifacts_size file_too_large! unless artifacts.size < max_artifacts_size(job)
expire_in = params['expire_in'] || expire_in = params['expire_in'] ||
Gitlab::CurrentSettings.current_application_settings.default_artifacts_expire_in Gitlab::CurrentSettings.current_application_settings.default_artifacts_expire_in
......
...@@ -954,4 +954,52 @@ describe Namespace do ...@@ -954,4 +954,52 @@ describe Namespace do
expect(group.has_parent?).to be_falsy expect(group.has_parent?).to be_falsy
end end
end end
describe '#closest_setting' do
using RSpec::Parameterized::TableSyntax
shared_examples_for 'fetching closest setting' do
let!(:root_namespace) { create(:namespace) }
let!(:namespace) { create(:namespace, parent: root_namespace) }
let(:setting) { namespace.closest_setting(setting_name) }
before do
root_namespace.update_attribute(setting_name, root_setting)
namespace.update_attribute(setting_name, child_setting)
end
it 'returns closest non-nil value' do
expect(setting).to eq(result)
end
end
context 'when setting is of non-boolean type' do
where(:root_setting, :child_setting, :result) do
100 | 200 | 200
100 | nil | 100
nil | nil | nil
end
with_them do
let(:setting_name) { :max_artifacts_size }
it_behaves_like 'fetching closest setting'
end
end
context 'when setting is of boolean type' do
where(:root_setting, :child_setting, :result) do
true | false | false
true | nil | true
nil | nil | nil
end
with_them do
let(:setting_name) { :lfs_enabled }
it_behaves_like 'fetching closest setting'
end
end
end
end end
...@@ -5121,6 +5121,53 @@ describe Project do ...@@ -5121,6 +5121,53 @@ describe Project do
end end
end end
describe '#closest_setting' do
using RSpec::Parameterized::TableSyntax
shared_examples_for 'fetching closest setting' do
let!(:namespace) { create(:namespace) }
let!(:project) { create(:project, namespace: namespace) }
let(:setting_name) { :some_setting }
let(:setting) { project.closest_setting(setting_name) }
before do
allow(project).to receive(:read_attribute).with(setting_name).and_return(project_setting)
allow(namespace).to receive(:closest_setting).with(setting_name).and_return(group_setting)
allow(Gitlab::CurrentSettings).to receive(setting_name).and_return(global_setting)
end
it 'returns closest non-nil value' do
expect(setting).to eq(result)
end
end
context 'when setting is of non-boolean type' do
where(:global_setting, :group_setting, :project_setting, :result) do
100 | 200 | 300 | 300
100 | 200 | nil | 200
100 | nil | nil | 100
nil | nil | nil | nil
end
with_them do
it_behaves_like 'fetching closest setting'
end
end
context 'when setting is of boolean type' do
where(:global_setting, :group_setting, :project_setting, :result) do
true | true | false | false
true | false | nil | false
true | nil | nil | true
end
with_them do
it_behaves_like 'fetching closest setting'
end
end
end
def rugged_config def rugged_config
rugged_repo(project.repository).config rugged_repo(project.repository).config
end end
......
...@@ -308,7 +308,9 @@ describe API::Runner, :clean_gitlab_redis_shared_state do ...@@ -308,7 +308,9 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
end end
describe '/api/v4/jobs' do describe '/api/v4/jobs' do
let(:project) { create(:project, shared_runners_enabled: false) } let(:root_namespace) { create(:namespace) }
let(:namespace) { create(:namespace, parent: root_namespace) }
let(:project) { create(:project, namespace: namespace, shared_runners_enabled: false) }
let(:pipeline) { create(:ci_pipeline_without_jobs, project: project, ref: 'master') } let(:pipeline) { create(:ci_pipeline_without_jobs, project: project, ref: 'master') }
let(:runner) { create(:ci_runner, :project, projects: [project]) } let(:runner) { create(:ci_runner, :project, projects: [project]) }
let(:job) do let(:job) do
...@@ -1412,15 +1414,57 @@ describe API::Runner, :clean_gitlab_redis_shared_state do ...@@ -1412,15 +1414,57 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
end end
end end
it 'fails to post too large artifact' do context 'when artifact is too large' do
stub_application_setting(max_artifacts_size: 0) let(:sample_max_size) { 100 }
authorize_artifacts_with_token_in_params(filesize: 100) shared_examples_for 'rejecting too large artifacts' do
it 'fails to post' do
authorize_artifacts_with_token_in_params(filesize: sample_max_size.megabytes.to_i)
expect(response).to have_gitlab_http_status(413) expect(response).to have_gitlab_http_status(413)
end end
end end
context 'based on application setting' do
before do
stub_application_setting(max_artifacts_size: sample_max_size)
end
it_behaves_like 'rejecting too large artifacts'
end
context 'based on root namespace setting' do
before do
stub_application_setting(max_artifacts_size: 200)
root_namespace.update!(max_artifacts_size: sample_max_size)
end
it_behaves_like 'rejecting too large artifacts'
end
context 'based on child namespace setting' do
before do
stub_application_setting(max_artifacts_size: 200)
root_namespace.update!(max_artifacts_size: 200)
namespace.update!(max_artifacts_size: sample_max_size)
end
it_behaves_like 'rejecting too large artifacts'
end
context 'based on project setting' do
before do
stub_application_setting(max_artifacts_size: 200)
root_namespace.update!(max_artifacts_size: 200)
namespace.update!(max_artifacts_size: 200)
project.update!(max_artifacts_size: sample_max_size)
end
it_behaves_like 'rejecting too large artifacts'
end
end
end
context 'when using token as header' do context 'when using token as header' do
it 'authorizes posting artifacts to running job' do it 'authorizes posting artifacts to running job' do
authorize_artifacts_with_token_in_headers authorize_artifacts_with_token_in_headers
......
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