Commit ba5089fa authored by Stan Hu's avatar Stan Hu

Log message when upload via API exceeds limit

Since Workhorse started accelerating attachments in the upload API
(https://gitlab.com/gitlab-org/gitlab/-/merge_requests/57250), the
`content_length` log in `api_json.log` no longer holds the actual bytes
used by the uploaded file. It now reports the number of bytes from the
Workhorse JSON metadata.

In order to determine which projects might be exceeding this limit, we
log a JSON warning that will make it easier to fill in the allow list
for https://gitlab.com/gitlab-org/gitlab/-/issues/325788.
parent 7e19d4ff
---
title: Log message when upload via API exceeds limit
merge_request: 57774
author:
type: added
...@@ -67,6 +67,16 @@ module API ...@@ -67,6 +67,16 @@ module API
PROJECT_ATTACHMENT_SIZE_EXEMPT PROJECT_ATTACHMENT_SIZE_EXEMPT
end end
# This is to help determine which projects to use in https://gitlab.com/gitlab-org/gitlab/-/issues/325788
def log_if_upload_exceed_max_size(user_project, file)
return if file.size <= user_project.max_attachment_size
return if exempt_from_global_attachment_size?(user_project)
if file.size > user_project.max_attachment_size
Gitlab::AppLogger.info({ message: "File exceeds maximum size", file_bytes: file.size, project_id: user_project.id, project_path: user_project.full_path })
end
end
end end
helpers do helpers do
...@@ -576,6 +586,8 @@ module API ...@@ -576,6 +586,8 @@ module API
requires :file, types: [Rack::Multipart::UploadedFile, ::API::Validations::Types::WorkhorseFile], desc: 'The attachment file to be uploaded' requires :file, types: [Rack::Multipart::UploadedFile, ::API::Validations::Types::WorkhorseFile], desc: 'The attachment file to be uploaded'
end end
post ":id/uploads", feature_category: :not_owned do post ":id/uploads", feature_category: :not_owned do
log_if_upload_exceed_max_size(user_project, params[:file])
service = UploadService.new(user_project, params[:file]) service = UploadService.new(user_project, params[:file])
service.override_max_attachment_size = project_attachment_size(user_project) service.override_max_attachment_size = project_attachment_size(user_project)
upload = service.execute upload = service.execute
......
...@@ -1519,6 +1519,8 @@ RSpec.describe API::Projects do ...@@ -1519,6 +1519,8 @@ RSpec.describe API::Projects do
end end
describe "POST /projects/:id/uploads" do describe "POST /projects/:id/uploads" do
let(:file) { fixture_file_upload("spec/fixtures/dk.png", "image/png") }
before do before do
project project
end end
...@@ -1528,7 +1530,7 @@ RSpec.describe API::Projects do ...@@ -1528,7 +1530,7 @@ RSpec.describe API::Projects do
expect(instance).to receive(:override_max_attachment_size=).with(project.max_attachment_size).and_call_original expect(instance).to receive(:override_max_attachment_size=).with(project.max_attachment_size).and_call_original
end end
post api("/projects/#{project.id}/uploads", user), params: { file: fixture_file_upload("spec/fixtures/dk.png", "image/png") } post api("/projects/#{project.id}/uploads", user), params: { file: file }
expect(response).to have_gitlab_http_status(:created) expect(response).to have_gitlab_http_status(:created)
expect(json_response['alt']).to eq("dk") expect(json_response['alt']).to eq("dk")
...@@ -1538,13 +1540,21 @@ RSpec.describe API::Projects do ...@@ -1538,13 +1540,21 @@ RSpec.describe API::Projects do
expect(json_response['full_path']).to start_with("/#{project.namespace.path}/#{project.path}/uploads") expect(json_response['full_path']).to start_with("/#{project.namespace.path}/#{project.path}/uploads")
end end
it "logs a warning if file exceeds attachment size" do
allow(Gitlab::CurrentSettings).to receive(:max_attachment_size).and_return(0)
expect(Gitlab::AppLogger).to receive(:info).with(hash_including(message: 'File exceeds maximum size')).and_call_original
post api("/projects/#{project.id}/uploads", user), params: { file: file }
end
shared_examples 'capped upload attachments' do shared_examples 'capped upload attachments' do
it "limits the upload to 1 GB" do it "limits the upload to 1 GB" do
expect_next_instance_of(UploadService) do |instance| expect_next_instance_of(UploadService) do |instance|
expect(instance).to receive(:override_max_attachment_size=).with(1.gigabyte).and_call_original expect(instance).to receive(:override_max_attachment_size=).with(1.gigabyte).and_call_original
end end
post api("/projects/#{project.id}/uploads", user), params: { file: fixture_file_upload("spec/fixtures/dk.png", "image/png") } post api("/projects/#{project.id}/uploads", user), params: { file: file }
expect(response).to have_gitlab_http_status(:created) expect(response).to have_gitlab_http_status(:created)
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