Commit 784612e2 authored by Erick Bajao's avatar Erick Bajao

Update artifact size authorization

Now that we have project and namespace level setting
for max_artifacts_size, we need to update the authorization
for it in some of the runner related API endpoints.

This also adds a new helper method `#closest_setting`
which fetches the closest non-nil value for a given setting name.
This will be useful for other settings like max_pages_size.
parent ed3d787e
...@@ -2250,8 +2250,22 @@ class Project < ApplicationRecord ...@@ -2250,8 +2250,22 @@ 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)
setting ||= Gitlab::CurrentSettings.send(name) # rubocop:disable GitlabSecurity/PublicSend
setting
end
private private
def closest_namespace_setting(name)
namespace
.self_and_ancestors
.detect { |n| !n.read_attribute(name).nil? }
.try(name)
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
......
...@@ -5121,6 +5121,60 @@ describe Project do ...@@ -5121,6 +5121,60 @@ describe Project do
end end
end end
describe '#closest_setting' do
let(:root_namespace) { create(:namespace) }
let(:namespace) { create(:namespace, parent: root_namespace) }
let(:project) { create(:project, namespace: namespace) }
let(:setting) { project.closest_setting(:max_artifacts_size) }
before do
stub_application_setting(max_artifacts_size: 100)
root_namespace.update!(max_artifacts_size: 200)
namespace.update!(max_artifacts_size: 300)
project.update!(max_artifacts_size: 400)
end
context 'when project has non-nil value for setting' do
it 'returns project level setting' do
expect(setting).to eq(400)
end
end
context 'when project has nil value for setting' do
before do
project.update!(max_artifacts_size: nil)
end
context 'and namespace has non-nil value for setting' do
it 'returns namespace level setting' do
expect(setting).to eq(300)
end
end
context 'and namespace has nil value for setting' do
before do
namespace.update!(max_artifacts_size: nil)
end
context 'and root namespace has non-nil value for setting' do
it 'returns root namespace level setting' do
expect(setting).to eq(200)
end
end
context 'and root namespace has nil value for setting' do
before do
root_namespace.update!(max_artifacts_size: nil)
end
it 'returns application level setting' do
expect(setting).to eq(100)
end
end
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