From fc715db48e1ecdb0bae9e99b50f6fc84874f9285 Mon Sep 17 00:00:00 2001 From: Jason Hall Date: Sun, 8 Jun 2025 16:17:43 -0400 Subject: [PATCH] Revert "feat: add configurable multi-part uploads to oci-distribution" --- Makefile | 12 -- src/registry/mod.rs | 6 +- src/registry/tests.rs | 15 --- vendor/oci-distribution/src/client.rs | 152 ++------------------------ 4 files changed, 9 insertions(+), 176 deletions(-) diff --git a/Makefile b/Makefile index 61e7fb8..f41ab0e 100644 --- a/Makefile +++ b/Makefile @@ -94,15 +94,3 @@ check-code: # Run all checks (format, lint, test) check: check-fmt lint test - -push-ttl: - @echo "Building and pushing to ttl.sh..." - KRUST_REPO=ttl.sh/jason cargo run build example/hello-krust - -push-gar: - @echo "Building and pushing to Google Artifact Registry..." - KRUST_REPO=us-central1-docker.pkg.dev/jason-chainguard/krust cargo run build example/hello-krust - -push-cgr: - @echo "Building and pushing to Chainguard registry..." - KRUST_REPO=cgr.dev/imjasonh.dev/krust cargo run build example/hello-krust diff --git a/src/registry/mod.rs b/src/registry/mod.rs index 5c060dd..8e9dbe5 100644 --- a/src/registry/mod.rs +++ b/src/registry/mod.rs @@ -14,11 +14,7 @@ pub struct RegistryClient { impl RegistryClient { pub fn new() -> Result { - let config = oci_distribution::client::ClientConfig { - use_chunked_uploads: false, // Disable chunked uploads for better compatibility with various registries - ..Default::default() - }; - let client = Client::new(config); + let client = Client::new(oci_distribution::client::ClientConfig::default()); Ok(Self { client }) } diff --git a/src/registry/tests.rs b/src/registry/tests.rs index 92bc81a..2a93475 100644 --- a/src/registry/tests.rs +++ b/src/registry/tests.rs @@ -24,19 +24,4 @@ mod tests { assert_eq!(repo, "myapp"); assert_eq!(tag, "v1.0"); } - - #[test] - fn test_registry_client_disables_chunked_uploads() { - // This test verifies that RegistryClient is created with chunked uploads disabled - // The actual verification happens in the constructor where we set - // config.use_chunked_uploads = false - let client = RegistryClient::new(); - assert!( - client.is_ok(), - "RegistryClient should be created successfully" - ); - - // The important part is that the client is configured correctly in new() - // to disable chunked uploads for better registry compatibility - } } diff --git a/vendor/oci-distribution/src/client.rs b/vendor/oci-distribution/src/client.rs index 036f7b1..011d652 100644 --- a/vendor/oci-distribution/src/client.rs +++ b/vendor/oci-distribution/src/client.rs @@ -478,18 +478,14 @@ impl Client { data: &[u8], digest: &str, ) -> Result { - if self.config.use_chunked_uploads { - match self.push_blob_chunked(image_ref, data, digest).await { - Ok(url) => Ok(url), - Err(OciDistributionError::SpecViolationError(violation)) => { - warn!(?violation, "Registry is not respecting the OCI Distribution Specification when doing chunked push operations"); - warn!("Attempting monolithic push"); - self.push_blob_monolithically(image_ref, data, digest).await - } - Err(e) => Err(e), + match self.push_blob_chunked(image_ref, data, digest).await { + Ok(url) => Ok(url), + Err(OciDistributionError::SpecViolationError(violation)) => { + warn!(?violation, "Registry is not respecting the OCI Distribution Specification when doing chunked push operations"); + warn!("Attempting monolithic push"); + self.push_blob_monolithically(image_ref, data, digest).await } - } else { - self.push_blob_monolithically(image_ref, data, digest).await + Err(e) => Err(e), } } @@ -1616,7 +1612,7 @@ impl<'a> RequestBuilderWrapper<'a> { fn from_client( client: &'a Client, f: impl Fn(&reqwest::Client) -> RequestBuilder, - ) -> RequestBuilderWrapper<'a> { + ) -> RequestBuilderWrapper { let request_builder = f(&client.client); RequestBuilderWrapper { client, @@ -1756,17 +1752,6 @@ pub struct ClientConfig { /// /// This defaults to [`DEFAULT_MAX_CONCURRENT_DOWNLOAD`]. pub max_concurrent_download: usize, - - /// Whether to use chunked uploads when pushing blobs. - /// - /// When enabled (default), the client will attempt to use chunked uploads - /// and fall back to monolithic uploads if the registry doesn't support - /// chunked uploads properly. - /// - /// When disabled, the client will always use monolithic uploads. - /// - /// This defaults to `true`. - pub use_chunked_uploads: bool, } impl Default for ClientConfig { @@ -1780,7 +1765,6 @@ impl Default for ClientConfig { platform_resolver: Some(Box::new(current_platform_resolver)), max_concurrent_upload: DEFAULT_MAX_CONCURRENT_UPLOAD, max_concurrent_download: DEFAULT_MAX_CONCURRENT_DOWNLOAD, - use_chunked_uploads: true, } } } @@ -3079,124 +3063,4 @@ mod test { .await .expect("Failed to pull manifest"); } - - #[tokio::test] - #[cfg(feature = "test-registry")] - async fn test_push_blob_with_chunked_disabled() { - let docker = clients::Cli::default(); - let test_container = docker.run(registry_image()); - let port = test_container.get_host_port_ipv4(5000); - - // Create a client with chunked uploads disabled - let c = Client::new(ClientConfig { - protocol: ClientProtocol::Http, - use_chunked_uploads: false, - ..Default::default() - }); - - let url = format!("localhost:{}/hello-wasm:v1", port); - let image: Reference = url.parse().unwrap(); - - c.auth(&image, &RegistryAuth::Anonymous, RegistryOperation::Push) - .await - .expect("result from auth request"); - - let image_data = b"test blob data for monolithic upload"; - let image_digest = format!("sha256:{}", sha256::digest(image_data)); - - // This should use monolithic upload since chunked is disabled - let location = c - .push_blob(&image, image_data, &image_digest) - .await - .expect("failed to push blob"); - - // Verify the blob was uploaded - assert_eq!( - location, - format!( - "http://localhost:{}/v2/hello-wasm/blobs/{}", - port, image_digest - ) - ); - } - - #[tokio::test] - #[cfg(feature = "test-registry")] - async fn test_push_blob_with_chunked_enabled() { - let docker = clients::Cli::default(); - let test_container = docker.run(registry_image()); - let port = test_container.get_host_port_ipv4(5000); - - // Create a client with chunked uploads explicitly enabled (default) - let c = Client::new(ClientConfig { - protocol: ClientProtocol::Http, - use_chunked_uploads: true, - ..Default::default() - }); - - let url = format!("localhost:{}/hello-wasm:v1", port); - let image: Reference = url.parse().unwrap(); - - c.auth(&image, &RegistryAuth::Anonymous, RegistryOperation::Push) - .await - .expect("result from auth request"); - - let image_data = b"test blob data for chunked upload"; - let image_digest = format!("sha256:{}", sha256::digest(image_data)); - - // This should attempt chunked upload first - let location = c - .push_blob(&image, image_data, &image_digest) - .await - .expect("failed to push blob"); - - // Verify the blob was uploaded - assert_eq!( - location, - format!( - "http://localhost:{}/v2/hello-wasm/blobs/{}", - port, image_digest - ) - ); - } - - #[tokio::test] - #[cfg(feature = "test-registry")] - async fn test_push_blob_fallback_behavior() { - let docker = clients::Cli::default(); - let test_container = docker.run(registry_image()); - let port = test_container.get_host_port_ipv4(5000); - - // Create a client with default config (chunked enabled) - let c = Client::new(ClientConfig { - protocol: ClientProtocol::Http, - ..Default::default() - }); - - let url = format!("localhost:{}/hello-wasm:v1", port); - let image: Reference = url.parse().unwrap(); - - c.auth(&image, &RegistryAuth::Anonymous, RegistryOperation::Push) - .await - .expect("result from auth request"); - - // Use a larger blob to ensure chunked upload is attempted - let image_data: Vec = (0..10000).map(|i| (i % 256) as u8).collect(); - let image_digest = format!("sha256:{}", sha256::digest(&image_data)); - - // This should attempt chunked upload and potentially fall back to monolithic - let location = c - .push_blob(&image, &image_data, &image_digest) - .await - .expect("failed to push blob"); - - // Verify the blob was uploaded - assert_eq!( - location, - format!( - "http://localhost:{}/v2/hello-wasm/blobs/{}", - port, image_digest - ) - ); - } }