-
Notifications
You must be signed in to change notification settings - Fork 1
Bundle-uri offload: publish base bundles at compaction, serve /git/{repo}.git/info/bundles #139
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,7 +20,7 @@ use git_cache_domain::{ | |
| UpstreamRefComparison, | ||
| }; | ||
| use git_cache_git::UploadPackProcess; | ||
| use git_cache_objectstore::lfs_object_key; | ||
| use git_cache_objectstore::{bundle_key, lfs_object_key}; | ||
| use http::{header, HeaderMap, Method, StatusCode, Uri}; | ||
| use serde::{Deserialize, Serialize}; | ||
| use sha2::{Digest, Sha256}; | ||
|
|
@@ -40,6 +40,11 @@ use tracing::{info, info_span, warn, Instrument}; | |
|
|
||
| const GIT_UPLOAD_PACK_STREAM_BUFFER_BYTES: usize = 64 * 1024; | ||
| const DIRECT_GIT_PROOF_TTL: Duration = Duration::from_secs(30); | ||
| /// Bundle list endpoint suffix for `git clone --bundle-uri`; the same path | ||
| /// with a trailing `/{sha256}.bundle` segment serves the bundle bytes when | ||
| /// the object store cannot presign URLs. | ||
| const BUNDLE_LIST_SUFFIX: &str = "/info/bundles"; | ||
| const BUNDLE_DOWNLOAD_INFIX: &str = "/info/bundles/"; | ||
| const PROXY_ON_MISS_HEADER: &str = "git-cache-use-proxy-on-miss"; | ||
| /// Disk reservation granularity for spooling a proxied upload-pack response | ||
| /// whose final size is unknown up front. | ||
|
|
@@ -780,7 +785,10 @@ async fn git_repo_inner(state: Arc<ApiState>, request: GitRepoRequest) -> Respon | |
| ))) | ||
| .into_response(); | ||
| } | ||
| GitRequestType::UploadPackRefs | GitRequestType::UploadPack => {} | ||
| GitRequestType::UploadPackRefs | ||
| | GitRequestType::UploadPack | ||
| | GitRequestType::BundleList | ||
| | GitRequestType::BundleDownload => {} | ||
| } | ||
|
|
||
| let started = Instant::now(); | ||
|
|
@@ -792,6 +800,8 @@ async fn git_repo_inner(state: Arc<ApiState>, request: GitRepoRequest) -> Respon | |
| match request_type { | ||
| GitRequestType::UploadPackRefs => git_repo_get(state, request, started, auth).await, | ||
| GitRequestType::UploadPack => git_repo_post(state, request, started, auth).await, | ||
| GitRequestType::BundleList => git_bundle_list(state, request, started, auth).await, | ||
| GitRequestType::BundleDownload => git_bundle_download(state, request, started, auth).await, | ||
| GitRequestType::ReceivePack | GitRequestType::LfsBatch | GitRequestType::LfsDownload => { | ||
| unreachable!("handled above") | ||
| } | ||
|
|
@@ -807,6 +817,8 @@ async fn git_repo_inner(state: Arc<ApiState>, request: GitRepoRequest) -> Respon | |
| enum GitRequestType { | ||
| UploadPackRefs, | ||
| UploadPack, | ||
| BundleList, | ||
| BundleDownload, | ||
| ReceivePack, | ||
| LfsBatch, | ||
| LfsDownload, | ||
|
|
@@ -836,6 +848,14 @@ fn git_request_type(request: &GitRepoRequest) -> GitRequestType { | |
| return GitRequestType::LfsDownload; | ||
| } | ||
|
|
||
| if request.method == Method::GET && path.ends_with(BUNDLE_LIST_SUFFIX) { | ||
| return GitRequestType::BundleList; | ||
| } | ||
|
|
||
| if request.method == Method::GET && path.contains(BUNDLE_DOWNLOAD_INFIX) { | ||
| return GitRequestType::BundleDownload; | ||
| } | ||
|
|
||
| if request.method == Method::GET | ||
| && path.ends_with("/info/refs") | ||
| && request | ||
|
|
@@ -1097,6 +1117,217 @@ async fn git_repo_post( | |
| } | ||
| } | ||
|
|
||
| /// Serve the bundle list for `git clone --bundle-uri`: a gitconfig-format | ||
| /// document pointing at the current head generation's base bundle, via a | ||
| /// presigned object-store URL when the backend supports it, otherwise via | ||
| /// this server's own bundle download path. | ||
| async fn git_bundle_list( | ||
| state: Arc<ApiState>, | ||
| request: GitRepoRequest, | ||
| started: Instant, | ||
| auth: UpstreamAuth, | ||
| ) -> Response { | ||
| let GitRepoRequest { | ||
| repo_path, | ||
| query: _, | ||
| headers, | ||
| method: _, | ||
| uri, | ||
| body: _, | ||
| request_id, | ||
| } = request; | ||
|
|
||
| let repo = match repo_from_git_path(&repo_path) { | ||
| Ok(repo) => repo, | ||
| Err(error) => return ApiError::from(error).into_response(), | ||
| }; | ||
|
|
||
| let materializer = Materializer::new(Arc::clone(&state.domain)); | ||
| if let Err(error) = materializer.validate_host(&repo) { | ||
| return ApiError::from(error).into_response(); | ||
| } | ||
| if !state.domain.config.bundle_uri.enabled { | ||
| return ApiError::from(GitCacheError::NotFound("bundle-uri is not enabled".into())) | ||
| .into_response(); | ||
| } | ||
|
|
||
| let materializer = materializer.using_upstream_auth(&auth); | ||
| if let Err(response) = | ||
| prove_direct_git_access(&state, &materializer, &repo, &auth, request_id).await | ||
| { | ||
| return response; | ||
| } | ||
|
|
||
| let bundle = match materializer.latest_bundle(&repo).await { | ||
| Ok(bundle) => bundle, | ||
| Err(error) => return ApiError::from(error).into_response(), | ||
| }; | ||
| let Some(bundle) = bundle else { | ||
| return ApiError::from(GitCacheError::NotFound(format!( | ||
| "no base bundle published for `{repo}`" | ||
| ))) | ||
| .into_response(); | ||
| }; | ||
|
|
||
| let ttl = Duration::from_secs(state.domain.config.bundle_uri.presign_ttl_secs); | ||
| let bundle_uri = match state.domain.store.presign_get(&bundle.key, ttl).await { | ||
| Ok(Some(url)) => url, | ||
| Ok(None) => match local_bundle_uri(&headers, &uri, &bundle.sha256) { | ||
| Ok(url) => url, | ||
| Err(error) => return ApiError::from(error).into_response(), | ||
| }, | ||
| Err(error) => return ApiError::from(error).into_response(), | ||
| }; | ||
|
|
||
| let body = format!( | ||
| "[bundle]\n\tversion = 1\n\tmode = all\n\n[bundle \"base\"]\n\turi = {bundle_uri}\n" | ||
| ); | ||
| info!( | ||
| request_id, | ||
| repo = %repo, | ||
| auth = auth_label(&auth), | ||
| bundle_key = %bundle.key, | ||
| bundle_len = bundle.len, | ||
| elapsed_ms = elapsed_ms(started), | ||
| status = %StatusCode::OK, | ||
| "direct git bundle list served" | ||
| ); | ||
| Response::builder() | ||
| .status(StatusCode::OK) | ||
| .header(header::CONTENT_TYPE, "text/plain; charset=utf-8") | ||
| .header(header::CACHE_CONTROL, "no-cache") | ||
| .body(Body::from(body)) | ||
| .expect("bundle list response") | ||
| } | ||
|
|
||
| /// Stream bundle bytes from the object store for backends that cannot mint | ||
| /// presigned URLs (the local filesystem store). The bundle is addressed by | ||
| /// its content sha, so the response is immutable. | ||
| async fn git_bundle_download( | ||
| state: Arc<ApiState>, | ||
| request: GitRepoRequest, | ||
| started: Instant, | ||
| auth: UpstreamAuth, | ||
| ) -> Response { | ||
| let request_id = request.request_id; | ||
| let path = request.uri.path().to_string(); | ||
|
|
||
| let repo = match repo_from_git_path(&request.repo_path) { | ||
| Ok(repo) => repo, | ||
| Err(error) => return ApiError::from(error).into_response(), | ||
| }; | ||
|
|
||
| let materializer = Materializer::new(Arc::clone(&state.domain)); | ||
| if let Err(error) = materializer.validate_host(&repo) { | ||
| return ApiError::from(error).into_response(); | ||
| } | ||
| if !state.domain.config.bundle_uri.enabled { | ||
| return ApiError::from(GitCacheError::NotFound("bundle-uri is not enabled".into())) | ||
| .into_response(); | ||
| } | ||
|
|
||
| let materializer = materializer.using_upstream_auth(&auth); | ||
| if let Err(response) = | ||
| prove_direct_git_access(&state, &materializer, &repo, &auth, request_id).await | ||
| { | ||
| return response; | ||
| } | ||
|
|
||
| let Some(sha256) = extract_bundle_sha_from_path(&path) else { | ||
| return ApiError::from(GitCacheError::Validation( | ||
| "bundle download path must end in /info/bundles/{sha256}.bundle".into(), | ||
| )) | ||
| .into_response(); | ||
| }; | ||
| let key = match bundle_key(&repo, sha256) { | ||
| Ok(key) => key, | ||
| Err(error) => return ApiError::from(error).into_response(), | ||
| }; | ||
|
|
||
| let max_bytes = state.domain.config.max_git_output_bytes as u64; | ||
| match state.domain.store.get_stream(&key).await { | ||
| Ok(Some((reader, len))) => { | ||
| info!( | ||
| request_id, | ||
| repo = %repo, | ||
| auth = auth_label(&auth), | ||
| bundle_key = %key, | ||
| bundle_len = len, | ||
| elapsed_ms = elapsed_ms(started), | ||
| status = %StatusCode::OK, | ||
| "direct git bundle download streaming" | ||
| ); | ||
| let capped_len = len.min(max_bytes); | ||
| let stream = ReaderStream::new(reader.take(max_bytes)); | ||
| Response::builder() | ||
| .status(StatusCode::OK) | ||
| .header(header::CONTENT_TYPE, "application/octet-stream") | ||
| .header(header::CONTENT_LENGTH, capped_len) | ||
| .body(Body::from_stream(stream)) | ||
| .expect("bundle download response") | ||
| } | ||
| Ok(None) => ApiError::from(GitCacheError::NotFound(format!( | ||
| "bundle `{sha256}` not found for `{repo}`" | ||
| ))) | ||
| .into_response(), | ||
| Err(error) => ApiError::from(error).into_response(), | ||
| } | ||
| } | ||
|
|
||
| /// Prove repo access for bundle endpoints with the same short-lived | ||
| /// `ls-remote` proof upload-pack uses, reusing a cached proof when present. | ||
| async fn prove_direct_git_access( | ||
| state: &Arc<ApiState>, | ||
| materializer: &Materializer, | ||
| repo: &RepoKey, | ||
| auth: &UpstreamAuth, | ||
| request_id: u64, | ||
| ) -> Result<(), Response> { | ||
| if state.direct_git_proofs.get(repo, auth).is_some() { | ||
| return Ok(()); | ||
| } | ||
| let proof_started = Instant::now(); | ||
| let comparison = match materializer.upstream_refs(repo).await { | ||
| Ok(comparison) => comparison, | ||
| Err(error) => return Err(ApiError::from(error).into_response()), | ||
| }; | ||
| info!( | ||
| request_id, | ||
| repo = %repo, | ||
| auth = auth_label(auth), | ||
| refs_count = comparison.all_upstream.len(), | ||
| elapsed_ms = elapsed_ms(proof_started), | ||
| "direct git bundle repo access proved" | ||
| ); | ||
| state.direct_git_proofs.insert(repo, auth, comparison); | ||
| Ok(()) | ||
| } | ||
|
|
||
| fn extract_bundle_sha_from_path(path: &str) -> Option<&str> { | ||
| let (_, after) = path.rsplit_once(BUNDLE_DOWNLOAD_INFIX)?; | ||
| let sha = after.strip_suffix(".bundle")?; | ||
| if sha.is_empty() || sha.contains('/') { | ||
| return None; | ||
| } | ||
| Some(sha) | ||
| } | ||
|
|
||
| /// Absolute URL of this server's own bundle download path, derived from the | ||
| /// incoming bundle list request. | ||
| fn local_bundle_uri(headers: &HeaderMap, uri: &Uri, sha256: &str) -> CoreResult<String> { | ||
| let host = headers | ||
| .get(header::HOST) | ||
| .and_then(|value| value.to_str().ok()) | ||
| .ok_or_else(|| { | ||
| GitCacheError::Validation("bundle list request is missing a Host header".into()) | ||
| })?; | ||
| let scheme = headers | ||
| .get("x-forwarded-proto") | ||
| .and_then(|value| value.to_str().ok()) | ||
| .unwrap_or("http"); | ||
| Ok(format!("{scheme}://{host}{}/{sha256}.bundle", uri.path())) | ||
| } | ||
|
Comment on lines
+1317
to
+1329
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔍 local_bundle_uri derives scheme/host from client-controlled headers
Was this helpful? React with 👍 or 👎 to provide feedback. Debug
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct — this fallback path only engages when the object store cannot presign (local filesystem store), and the echoed scheme/host only affects the requesting client's own clone. Production S3 deployments hand out presigned URLs and never hit this branch. For local-store deployments behind TLS-terminating proxies, the proxy needs to set |
||
|
|
||
| fn upstream_api_auth(headers: &HeaderMap) -> Result<UpstreamAuth, ApiError> { | ||
| parse_optional_upstream_auth_header(headers, "git-cache-upstream-authorization") | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 Large history bundles are served truncated and corrupt to clients
The downloaded history bundle is cut off at the small git-output byte limit (
reader.take(max_bytes)atcrates/git-cache-api/src/lib.rs:1261) before it is sent, so any repository whose bundle is larger than that limit is delivered incomplete and unusable.Impact: Clients cloning with bundle-uri against a filesystem-backed deployment get a truncated, corrupt bundle whenever a repo's history exceeds ~16 MiB, so the bundle fails verification and the accelerated clone breaks.
Wrong byte bound applied to content-addressed bundle streaming
git_bundle_downloadreadsmax_bytes = state.domain.config.max_git_output_bytes(crates/git-cache-api/src/lib.rs:1247), whose default is 16 MiB (crates/git-cache-core/src/config.rs:373-375). It then streamsreader.take(max_bytes)and setsContent-Lengthtolen.min(max_bytes)(crates/git-cache-api/src/lib.rs:1260-1265). A base bundle produced bycreate_compaction_bundlecarries the full history objects of the cached upstream branch tips (crates/git-cache-domain/src/materializer/generations.rs:1008-1082), which routinely exceeds 16 MiB. The download path is only used for object stores that cannot presign (the local filesystem store; S3 presigns and bypasses this handler), so local-store deployments withbundle_uri.enabledwill hand clients a silently truncated bundle. Compare the LFS download path, which correctly bounds bylfs.max_object_bytes(2 GiB default) atcrates/git-cache-api/src/lib.rs:2942-2955. Because the bundle object is content-addressed and its exact lengthlenis already known fromget_stream, the stream should be bounded bylenrather than the unrelated git-subprocess output limit.Prompt for agents
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
Playground
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 6ce3d2c — the stream is now bounded by the object's own length (
reader.take(len)withContent-Length: len) instead ofmax_git_output_bytes, so large bundles are no longer truncated. The bound is exact because the bundle is a content-addressed object whose size is returned byget_stream.