From 0c621a27ee9d3a846a3501eb43fb5c546985e797 Mon Sep 17 00:00:00 2001 From: MasterPtato Date: Fri, 22 Nov 2024 01:02:46 +0000 Subject: [PATCH] fix: remove multipart flag --- packages/api/actor/src/route/actors.rs | 2 +- packages/api/actor/src/route/builds.rs | 1 - packages/api/cloud/src/route/games/builds.rs | 31 +---- packages/services/build/ops/create/src/lib.rs | 1 - packages/services/build/proto/create.proto | 3 +- .../standalone/default-create/src/lib.rs | 1 - .../services/upload/ops/prepare/src/lib.rs | 122 ++++++++---------- .../upload/ops/prepare/tests/integration.rs | 1 - resources/legacy/proto/backend/upload.proto | 3 +- 9 files changed, 61 insertions(+), 104 deletions(-) diff --git a/packages/api/actor/src/route/actors.rs b/packages/api/actor/src/route/actors.rs index 3877104284..2d1c4e2293 100644 --- a/packages/api/actor/src/route/actors.rs +++ b/packages/api/actor/src/route/actors.rs @@ -409,7 +409,7 @@ pub async fn upgrade( assert::server_for_env(&ctx, actor_id, game_id, env_id).await?; - let build_id = resolve_build_id(&ctx, game_id, body.build, body.build_tags.flatten()).await?; + let build_id = resolve_build_id(&ctx, env_id, body.build, body.build_tags.flatten()).await?; let mut sub = ctx .subscribe::(("server_id", actor_id)) diff --git a/packages/api/actor/src/route/builds.rs b/packages/api/actor/src/route/builds.rs index f9f92a0657..2da4f4e9e5 100644 --- a/packages/api/actor/src/route/builds.rs +++ b/packages/api/actor/src/route/builds.rs @@ -279,7 +279,6 @@ pub async fn create_build( display_name: body.name, image_tag: Some(image_tag), image_file: Some((*body.image_file).api_try_into()?), - multipart: multipart_upload, kind: kind as i32, compression: compression as i32, }) diff --git a/packages/api/cloud/src/route/games/builds.rs b/packages/api/cloud/src/route/games/builds.rs index ccb8cbee0f..17072e3a1d 100644 --- a/packages/api/cloud/src/route/games/builds.rs +++ b/packages/api/cloud/src/route/games/builds.rs @@ -86,8 +86,6 @@ pub async fn create_build( // TODO: Read and validate image file - let multipart_upload = body.multipart_upload.unwrap_or(false); - let kind = match body.kind { None | Some(models::CloudGamesBuildKind::DockerImage) => { backend::build::BuildKind::DockerImage @@ -107,40 +105,23 @@ pub async fn create_build( display_name: body.display_name, image_tag: Some(body.image_tag), image_file: Some((*body.image_file).api_try_into()?), - multipart: multipart_upload, kind: kind as i32, compression: compression as i32, ..Default::default() }) .await?; - let image_presigned_request = if !multipart_upload { - Some(Box::new( - unwrap!(create_res.image_presigned_requests.first()) - .clone() - .api_try_into()?, - )) - } else { - None - }; - - let image_presigned_requests = if multipart_upload { - Some( + Ok(models::CloudGamesCreateGameBuildResponse { + build_id: unwrap_ref!(create_res.build_id).as_uuid(), + upload_id: unwrap_ref!(create_res.upload_id).as_uuid(), + image_presigned_request: None, + image_presigned_requests: Some( create_res .image_presigned_requests .iter() .cloned() .map(ApiTryInto::api_try_into) .collect::>>()?, - ) - } else { - None - }; - - Ok(models::CloudGamesCreateGameBuildResponse { - build_id: unwrap_ref!(create_res.build_id).as_uuid(), - upload_id: unwrap_ref!(create_res.upload_id).as_uuid(), - image_presigned_request, - image_presigned_requests, + ), }) } diff --git a/packages/services/build/ops/create/src/lib.rs b/packages/services/build/ops/create/src/lib.rs index f1ea9246f8..ca100e15d9 100644 --- a/packages/services/build/ops/create/src/lib.rs +++ b/packages/services/build/ops/create/src/lib.rs @@ -87,7 +87,6 @@ async fn handle( backend::upload::PrepareFile { path: file_name, content_length: image_file.content_length, - multipart: ctx.multipart, ..Default::default() }, ], diff --git a/packages/services/build/proto/create.proto b/packages/services/build/proto/create.proto index d9ab175d9f..53f3c11c3c 100644 --- a/packages/services/build/proto/create.proto +++ b/packages/services/build/proto/create.proto @@ -7,12 +7,13 @@ import "resources/legacy/proto/backend/upload.proto"; import "resources/legacy/proto/backend/build.proto"; message Request { + reserved 6; + optional rivet.common.Uuid game_id = 1; optional rivet.common.Uuid env_id = 10; string display_name = 2; optional string image_tag = 4; optional rivet.backend.upload.PrepareFile image_file = 3; - bool multipart = 6; rivet.backend.build.BuildKind kind = 7; rivet.backend.build.BuildCompression compression = 8; map tags = 9; // JSON diff --git a/packages/services/build/standalone/default-create/src/lib.rs b/packages/services/build/standalone/default-create/src/lib.rs index b55fa8c96c..78c2f0114b 100644 --- a/packages/services/build/standalone/default-create/src/lib.rs +++ b/packages/services/build/standalone/default-create/src/lib.rs @@ -97,7 +97,6 @@ async fn upload_build( backend::upload::PrepareFile { path: unwrap!(unwrap!(build.key.file_name(), "should have file name").to_str()).to_string(), content_length: len, - multipart: len > util::file_size::mebibytes(100), ..Default::default() }, ], diff --git a/packages/services/upload/ops/prepare/src/lib.rs b/packages/services/upload/ops/prepare/src/lib.rs index 7ad260a0db..89c85c2878 100644 --- a/packages/services/upload/ops/prepare/src/lib.rs +++ b/packages/services/upload/ops/prepare/src/lib.rs @@ -6,8 +6,13 @@ use rivet_operation::prelude::*; use serde_json::json; pub const CHUNK_SIZE: u64 = util::file_size::mebibytes(100); -const MAX_UPLOAD_SIZE: u64 = util::file_size::gigabytes(10); -const MAX_MULTIPART_UPLOAD_SIZE: u64 = util::file_size::gigabytes(100); +const MAX_UPLOAD_SIZE: u64 = util::file_size::gigabytes(100); +/// Minimum size for AWS multipart file uploads. +/// +/// See AWS error code `EntityTooSmall` +/// +/// https://docs.aws.amazon.com/AmazonS3/latest/API/API_CompleteMultipartUpload.html +const MIN_MULTIPART_FILE_SIZE: u64 = util::file_size::mebibytes(5); struct PrepareResult { multipart: Option, @@ -45,24 +50,11 @@ async fn handle( .await?; // Validate upload sizes - let total_content_length = ctx - .files - .iter() - .filter(|file| !file.multipart) - .fold(0, |acc, x| acc + x.content_length); - let total_multipart_content_length = ctx - .files - .iter() - .filter(|file| file.multipart) - .fold(0, |acc, x| acc + x.content_length); - tracing::info!(len=%ctx.files.len(), %total_content_length, %total_multipart_content_length, "file info"); + let total_content_length = ctx.files.iter().fold(0, |acc, x| acc + x.content_length); + tracing::info!(len=%ctx.files.len(), %total_content_length, "file info"); ensure!( total_content_length < MAX_UPLOAD_SIZE, - "upload size must be < 10 gb" - ); - ensure!( - total_content_length < MAX_MULTIPART_UPLOAD_SIZE, - "multipart uploads must be < 100 gb" + "uploads must be < 100 gb" ); let user_id = ctx.user_id.map(|x| x.as_uuid()); @@ -134,12 +126,8 @@ async fn handle( let s3_client_internal = s3_client_internal.clone(); let s3_client_external = s3_client_external.clone(); - if file.multipart { - handle_multipart_upload(s3_client_internal, s3_client_external, upload_id, file) - .boxed() - } else { - handle_upload(s3_client_external, upload_id, file).boxed() - } + handle_multipart_upload(s3_client_internal, s3_client_external, upload_id, file) + .boxed() }) .buffer_unordered(16) .try_collect::>() @@ -196,7 +184,6 @@ async fn handle( "bucket": ctx.bucket, "files": ctx.files.len(), "total_content_length": total_content_length, - "total_multipart_content_length": total_multipart_content_length, }))?), ..Default::default() } @@ -210,52 +197,6 @@ async fn handle( }) } -async fn handle_upload( - s3_client_external: s3_util::Client, - upload_id: Uuid, - file: backend::upload::PrepareFile, -) -> GlobalResult> { - let fut = async move { - // Sign an upload request - let mut put_obj_builder = s3_client_external - .put_object() - .bucket(s3_client_external.bucket()) - .key(format!("{}/{}", upload_id, file.path)) - .content_length(file.content_length as i64); - if let Some(mime) = &file.mime { - put_obj_builder = put_obj_builder.content_type(mime.clone()); - } - let presigned_upload_req = put_obj_builder - .presigned( - s3_util::aws_sdk_s3::presigning::PresigningConfig::builder() - .expires_in(Duration::from_secs(60 * 60)) - .build()?, - ) - .await?; - - GlobalResult::Ok(backend::upload::PresignedUploadRequest { - path: file.path.clone(), - url: presigned_upload_req.uri().to_string(), - part_number: 0, - byte_offset: 0, - content_length: file.content_length, - }) - } - .boxed(); - - Ok(vec![PrepareResult { - multipart: None, - fut, - }]) -} - -/// Minimum size for AWS multipart file uploads. -/// -/// See AWS error code `EntityTooSmall` -/// -/// https://docs.aws.amazon.com/AmazonS3/latest/API/API_CompleteMultipartUpload.html -const MIN_MULTIPART_FILE_SIZE: u64 = util::file_size::mebibytes(5); - async fn handle_multipart_upload( s3_client_internal: s3_util::Client, s3_client_external: s3_util::Client, @@ -331,3 +272,42 @@ async fn handle_multipart_upload( }) .collect::>()) } + +async fn handle_upload( + s3_client_external: s3_util::Client, + upload_id: Uuid, + file: backend::upload::PrepareFile, +) -> GlobalResult> { + let fut = async move { + // Sign an upload request + let mut put_obj_builder = s3_client_external + .put_object() + .bucket(s3_client_external.bucket()) + .key(format!("{}/{}", upload_id, file.path)) + .content_length(file.content_length as i64); + if let Some(mime) = &file.mime { + put_obj_builder = put_obj_builder.content_type(mime.clone()); + } + let presigned_upload_req = put_obj_builder + .presigned( + s3_util::aws_sdk_s3::presigning::PresigningConfig::builder() + .expires_in(Duration::from_secs(60 * 60)) + .build()?, + ) + .await?; + + GlobalResult::Ok(backend::upload::PresignedUploadRequest { + path: file.path.clone(), + url: presigned_upload_req.uri().to_string(), + part_number: 0, + byte_offset: 0, + content_length: file.content_length, + }) + } + .boxed(); + + Ok(vec![PrepareResult { + multipart: None, + fut, + }]) +} diff --git a/packages/services/upload/ops/prepare/tests/integration.rs b/packages/services/upload/ops/prepare/tests/integration.rs index 0286ab4291..4cf8649497 100644 --- a/packages/services/upload/ops/prepare/tests/integration.rs +++ b/packages/services/upload/ops/prepare/tests/integration.rs @@ -152,7 +152,6 @@ async fn multipart(ctx: TestCtx) { .fold(0, |acc, x| { acc + x.len() }) as u64, - multipart: true, ..Default::default() }], }) diff --git a/resources/legacy/proto/backend/upload.proto b/resources/legacy/proto/backend/upload.proto index 75f3986bd7..5488da845b 100644 --- a/resources/legacy/proto/backend/upload.proto +++ b/resources/legacy/proto/backend/upload.proto @@ -39,12 +39,11 @@ message UploadFile { } message PrepareFile { - reserved 4; + reserved 4, 5; string path = 1; optional string mime = 2; uint64 content_length = 3; - bool multipart = 5; } message PresignedUploadRequest {