From 8a644f0ffd07553120a89361d34ea609922b6a71 Mon Sep 17 00:00:00 2001 From: Kornel Date: Thu, 22 May 2025 11:46:42 +0100 Subject: [PATCH 1/7] Rename --- crates/freighter/src/{main.rs => lib.rs} | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) rename crates/freighter/src/{main.rs => lib.rs} (96%) diff --git a/crates/freighter/src/main.rs b/crates/freighter/src/lib.rs similarity index 96% rename from crates/freighter/src/main.rs rename to crates/freighter/src/lib.rs index 4588b4e..38b4af7 100644 --- a/crates/freighter/src/main.rs +++ b/crates/freighter/src/lib.rs @@ -1,5 +1,4 @@ use anyhow::Context; -use clap::Parser; cfg_if::cfg_if! { if #[cfg(feature = "filesystem-index-backend")] { @@ -26,15 +25,10 @@ use metrics_exporter_prometheus::PrometheusBuilder; use std::fs::read_to_string; use tokio::net::TcpListener; -mod cli; +pub mod cli; mod config; -#[tokio::main] -async fn main() -> anyhow::Result<()> { - tracing_subscriber::fmt::init(); - - let args = cli::FreighterArgs::parse(); - +pub async fn run(args: cli::FreighterArgs) -> anyhow::Result<()> { let config: config::Config = serde_yaml::from_str( &read_to_string(args.config) .context("Failed to read config file from disk, is it present?")?, From 8c88fcdbe11f58bb1d69cbf2b6d50be69ce3af1b Mon Sep 17 00:00:00 2001 From: Kornel Date: Thu, 22 May 2025 11:50:32 +0100 Subject: [PATCH 2/7] Separate main --- crates/freighter/src/main.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 crates/freighter/src/main.rs diff --git a/crates/freighter/src/main.rs b/crates/freighter/src/main.rs new file mode 100644 index 0000000..2c5ef5d --- /dev/null +++ b/crates/freighter/src/main.rs @@ -0,0 +1,10 @@ +use clap::Parser; + +#[tokio::main] +async fn main() -> anyhow::Result<()> { + tracing_subscriber::fmt::init(); + + let args = freighter::cli::FreighterArgs::parse(); + + freighter::run(args).await +} From d2871e0c651d48539566537ee6c126a79a508e40 Mon Sep 17 00:00:00 2001 From: Kornel Date: Thu, 22 May 2025 12:11:58 +0100 Subject: [PATCH 3/7] Better config loading error --- crates/freighter/src/lib.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/crates/freighter/src/lib.rs b/crates/freighter/src/lib.rs index 38b4af7..e071b37 100644 --- a/crates/freighter/src/lib.rs +++ b/crates/freighter/src/lib.rs @@ -29,11 +29,18 @@ pub mod cli; mod config; pub async fn run(args: cli::FreighterArgs) -> anyhow::Result<()> { - let config: config::Config = serde_yaml::from_str( - &read_to_string(args.config) - .context("Failed to read config file from disk, is it present?")?, - ) - .context("Failed to deserialize config file, please make sure its in the right format")?; + let config: config::Config = + serde_yaml::from_str(&read_to_string(&args.config).with_context(|| { + format!( + "Failed to read config file from disk, is {} present in {}?", + args.config.display(), + std::env::current_dir() + .as_deref() + .unwrap_or(".".as_ref()) + .display() + ) + })?) + .context("Failed to deserialize config file, please make sure its in the right format")?; let config::Config { service, From 967b69ba8e0cfd280ee2ef7a1d01ae117dc2ba40 Mon Sep 17 00:00:00 2001 From: Kornel Date: Thu, 22 May 2025 12:12:51 +0100 Subject: [PATCH 4/7] Separate listening from serving future --- crates/freighter/src/lib.rs | 18 +++++++++++------- crates/freighter/src/main.rs | 2 +- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/crates/freighter/src/lib.rs b/crates/freighter/src/lib.rs index e071b37..bacad8e 100644 --- a/crates/freighter/src/lib.rs +++ b/crates/freighter/src/lib.rs @@ -28,7 +28,9 @@ use tokio::net::TcpListener; pub mod cli; mod config; -pub async fn run(args: cli::FreighterArgs) -> anyhow::Result<()> { +pub async fn start_listening( + args: cli::FreighterArgs, +) -> anyhow::Result>> { let config: config::Config = serde_yaml::from_str(&read_to_string(&args.config).with_context(|| { format!( @@ -95,14 +97,16 @@ pub async fn run(args: cli::FreighterArgs) -> anyhow::Result<()> { ); let listener = TcpListener::bind(addr).await?; - axum::serve(listener, router.into_make_service()) - .with_graceful_shutdown(shutdown_signal()) - .await - .context("Freighter server exited with error")?; - tracing::info!("Completed graceful shutdown"); + return Ok(async move { + axum::serve(listener, router.into_make_service()) + .with_graceful_shutdown(shutdown_signal()) + .await + .context("Freighter server exited with error")?; - Ok(()) + tracing::info!("Completed graceful shutdown"); + Ok(()) + }); } // Based on: https://github.com/tokio-rs/axum/blob/main/examples/graceful-shutdown/src/main.rs diff --git a/crates/freighter/src/main.rs b/crates/freighter/src/main.rs index 2c5ef5d..315f7a4 100644 --- a/crates/freighter/src/main.rs +++ b/crates/freighter/src/main.rs @@ -6,5 +6,5 @@ async fn main() -> anyhow::Result<()> { let args = freighter::cli::FreighterArgs::parse(); - freighter::run(args).await + freighter::start_listening(args).await?.await } From 4ca7e4da24a401f231aab104861c19292b78e5f5 Mon Sep 17 00:00:00 2001 From: Kornel Date: Thu, 22 May 2025 12:56:21 +0100 Subject: [PATCH 5/7] Test with real Cargo as the client --- Cargo.lock | 2 + configs/local.s3-based.yaml | 6 +- crates/freighter/Cargo.toml | 4 ++ crates/freighter/tests/cargo.rs | 115 ++++++++++++++++++++++++++++++++ 4 files changed, 124 insertions(+), 3 deletions(-) create mode 100644 crates/freighter/tests/cargo.rs diff --git a/Cargo.lock b/Cargo.lock index 7bbeea2..28c3320 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1149,8 +1149,10 @@ dependencies = [ "freighter-server", "freighter-storage", "metrics-exporter-prometheus", + "rand 0.9.0", "serde", "serde_yaml", + "tempfile", "tokio", "tracing", "tracing-subscriber", diff --git a/configs/local.s3-based.yaml b/configs/local.s3-based.yaml index 3a4e089..7a9394d 100644 --- a/configs/local.s3-based.yaml +++ b/configs/local.s3-based.yaml @@ -1,7 +1,7 @@ service: address: 0.0.0.0:8080 - download_endpoint: 127.0.0.1:8080/downloads/{crate}/{version} - api_endpoint: 127.0.0.1:8080 + download_endpoint: http://127.0.0.1:8080/downloads/{crate}/{version} + api_endpoint: http://127.0.0.1:8080 metrics_address: 0.0.0.0:8081 auth_required: false @@ -19,4 +19,4 @@ store: access_key_id: "1234567890" access_key_secret: "valid-secret" -auth_allow_full_access_without_any_checks: true \ No newline at end of file +auth_allow_full_access_without_any_checks: true diff --git a/crates/freighter/Cargo.toml b/crates/freighter/Cargo.toml index fa7be6b..8224d92 100644 --- a/crates/freighter/Cargo.toml +++ b/crates/freighter/Cargo.toml @@ -40,3 +40,7 @@ yes-auth-backend = ["freighter-auth/yes-backend"] [lints] workspace = true + +[dev-dependencies] +rand.workspace = true +tempfile.workspace = true diff --git a/crates/freighter/tests/cargo.rs b/crates/freighter/tests/cargo.rs new file mode 100644 index 0000000..7e43ed4 --- /dev/null +++ b/crates/freighter/tests/cargo.rs @@ -0,0 +1,115 @@ +// Run with: +// cargo t -F filesystem-index-backend,yes-auth-backend +// +// This test doesn't set up any auth, so it won't work if proper auth is enabled +#[cfg(feature = "yes-auth-backend")] +#[cfg(feature = "filesystem-index-backend")] +#[cfg(not(feature = "filesystem-auth-backend"))] +#[cfg(not(feature = "cloudflare-auth-backend"))] +#[tokio::test] +async fn cargo_client_yes_auth_backend() { + let listener = freighter::start_listening(freighter::cli::FreighterArgs { + config: "../../configs/local.s3-based.yaml".into(), + }) + .await + .unwrap(); + + let server_handle = tokio::task::spawn(listener).abort_handle(); + + tokio::task::spawn_blocking(move || { + test_cargo_publish(); + server_handle.abort() + }) + .await + .unwrap(); +} + +fn test_cargo_publish() { + let tempdir = tempfile::tempdir().unwrap(); + let path = tempdir.path(); + + let random_id: u128 = rand::random(); + let test_crate1_name = format!("test_crate1_{random_id:x}"); + + let cargo_dir = path.join(".cargo"); + std::fs::create_dir(&cargo_dir).unwrap(); + let cargo_config = cargo_dir.join("config.toml"); + std::fs::write( + &cargo_config, + format!( + r#" +[registries.test_registry] +index = "sparse+http://127.0.0.1:8080/index/" + "# + ), + ) + .unwrap(); + + let test_crate1 = path.join("test_crate1"); + let src = test_crate1.join("src"); + std::fs::create_dir_all(&test_crate1.join("src")).unwrap(); + + std::fs::write( + &src.join("lib.rs"), + format!("pub const ID_{random_id}: bool = true;"), + ) + .unwrap(); + std::fs::write( + &test_crate1.join("Cargo.toml"), + format!( + r#" +[package] +name = "{test_crate1_name}" +edition = "2024" +version = "1.0.0" +description = "test" +license = "MIT" +publish = ["test_registry"] +"# + ), + ) + .unwrap(); + + let res = std::process::Command::new("cargo") + .arg("publish") + .current_dir(&test_crate1) + .env("CARGO_REGISTRIES_TEST_REGISTRY_TOKEN", "ok") + .status() + .unwrap(); + assert!(res.success()); + + let test_crate2 = path.join("test_crate2"); + let src = test_crate2.join("src"); + std::fs::create_dir_all(&test_crate2.join("src")).unwrap(); + + std::fs::write( + &src.join("lib.rs"), + format!("pub use {test_crate1_name}::ID_{random_id};"), + ) + .unwrap(); + std::fs::write( + &test_crate2.join("Cargo.toml"), + format!( + r#" +[package] +name = "test_crate2" +edition = "2024" +version = "1.0.0" +description = "test" +license = "MIT" +publish = ["test_registry"] + +[dependencies] +{test_crate1_name} = {{ version = "1", registry = "test_registry" }} + "# + ), + ) + .unwrap(); + + let res = std::process::Command::new("cargo") + .current_dir(&test_crate2) + .arg("b") + .status() + .unwrap(); + assert!(res.success()); +} From bef3bd7bde80e4b1ab88d396f622c9b99cf81bc2 Mon Sep 17 00:00:00 2001 From: Kornel Date: Tue, 27 May 2025 14:13:45 +0100 Subject: [PATCH 6/7] Support logging in tests --- Cargo.lock | 1 + crates/freighter-auth/Cargo.toml | 1 + crates/freighter-auth/src/fs_backend.rs | 2 ++ crates/freighter-server/Cargo.toml | 3 ++- crates/freighter-server/tests/api.rs | 8 ++++++++ crates/freighter-server/tests/e2e.rs | 2 ++ crates/freighter-server/tests/index.rs | 6 ++++++ crates/freighter/tests/cargo.rs | 2 ++ 8 files changed, 24 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 28c3320..87fad15 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1199,6 +1199,7 @@ dependencies = [ "thiserror 2.0.12", "tokio", "tracing", + "tracing-subscriber", ] [[package]] diff --git a/crates/freighter-auth/Cargo.toml b/crates/freighter-auth/Cargo.toml index b0c5ec4..1166d70 100644 --- a/crates/freighter-auth/Cargo.toml +++ b/crates/freighter-auth/Cargo.toml @@ -44,6 +44,7 @@ parking_lot = { version = "0.12.3", optional = true } [dev-dependencies] tokio = { workspace = true, features = ["macros", "rt"] } +tracing-subscriber = { workspace = true } [lints] workspace = true diff --git a/crates/freighter-auth/src/fs_backend.rs b/crates/freighter-auth/src/fs_backend.rs index c416d97..fcbb3b1 100644 --- a/crates/freighter-auth/src/fs_backend.rs +++ b/crates/freighter-auth/src/fs_backend.rs @@ -290,6 +290,8 @@ impl fmt::Debug for HashedToken { #[cfg(test)] #[tokio::test] async fn test_fs_tokens() { + let _ = tracing_subscriber::fmt::fmt().with_test_writer().try_init(); + let dir = tempfile::tempdir().unwrap(); let auth = FsAuthProvider::new(Config { auth_path: dir.path().to_path_buf(), auth_tokens_pepper: [123; 18] }).unwrap(); let user1 = auth.register("user1").await.unwrap(); diff --git a/crates/freighter-server/Cargo.toml b/crates/freighter-server/Cargo.toml index e8f597d..5cf510e 100644 --- a/crates/freighter-server/Cargo.toml +++ b/crates/freighter-server/Cargo.toml @@ -54,7 +54,8 @@ freighter-api-types = { workspace = true, features = ["client"] } async-trait = { workspace = true } hyper = { workspace = true } tower = { workspace = true } -tempfile.workspace = true +tempfile = { workspace = true } +tracing-subscriber = { workspace = true } [lints] workspace = true diff --git a/crates/freighter-server/tests/api.rs b/crates/freighter-server/tests/api.rs index 065d226..3f8270e 100644 --- a/crates/freighter-server/tests/api.rs +++ b/crates/freighter-server/tests/api.rs @@ -14,6 +14,8 @@ use tower::ServiceExt; #[tokio::test] async fn publish_crate() { + let _ = tracing_subscriber::fmt::fmt().with_test_writer().try_init(); + let router = api::api_router(); const TOKEN: &str = "12345"; @@ -44,6 +46,8 @@ async fn publish_crate() { #[tokio::test] async fn publish_crate_auth_denied() { + let _ = tracing_subscriber::fmt::fmt().with_test_writer().try_init(); + let router = api::api_router(); let state = ServiceStateBuilder::default() @@ -72,6 +76,8 @@ async fn publish_crate_auth_denied() { #[tokio::test] async fn index_auth() { + let _ = tracing_subscriber::fmt::fmt().with_test_writer().try_init(); + let state = ServiceStateBuilder::default() .auth_required(true) .index_provider(MockIndexProvider { @@ -114,6 +120,8 @@ async fn index_auth() { #[tokio::test] async fn list_all_crates() { + let _ = tracing_subscriber::fmt::fmt().with_test_writer().try_init(); + let crates = BTreeMap::from([ ( "example-lib".to_owned(), diff --git a/crates/freighter-server/tests/e2e.rs b/crates/freighter-server/tests/e2e.rs index 5a75831..e82ee22 100644 --- a/crates/freighter-server/tests/e2e.rs +++ b/crates/freighter-server/tests/e2e.rs @@ -37,6 +37,8 @@ struct TestServerConfig { impl TestServerConfig { fn from_env(default_port: u16) -> Self { + let _ = tracing_subscriber::fmt::fmt().with_test_writer().try_init(); + Self { db: Config { user: Some(var("POSTGRES_USER").unwrap_or("freighter".to_owned())), diff --git a/crates/freighter-server/tests/index.rs b/crates/freighter-server/tests/index.rs index 518897a..16f7284 100644 --- a/crates/freighter-server/tests/index.rs +++ b/crates/freighter-server/tests/index.rs @@ -12,6 +12,8 @@ use tower::ServiceExt; #[tokio::test] async fn index_config_endpoint() { + let _ = tracing_subscriber::fmt::fmt().with_test_writer().try_init(); + let router = index::index_router(); let state = ServiceStateBuilder::default().build(); @@ -42,6 +44,8 @@ async fn index_config_endpoint() { #[tokio::test] async fn missing_crate() { + let _ = tracing_subscriber::fmt::fmt().with_test_writer().try_init(); + let router = index::index_router(); let state = ServiceStateBuilder::default().build(); @@ -62,6 +66,8 @@ async fn missing_crate() { #[tokio::test] async fn valid_crate() { + let _ = tracing_subscriber::fmt::fmt().with_test_writer().try_init(); + let router = index::index_router(); let crates = BTreeMap::from([( diff --git a/crates/freighter/tests/cargo.rs b/crates/freighter/tests/cargo.rs index 7e43ed4..6b81e41 100644 --- a/crates/freighter/tests/cargo.rs +++ b/crates/freighter/tests/cargo.rs @@ -8,6 +8,8 @@ #[cfg(not(feature = "cloudflare-auth-backend"))] #[tokio::test] async fn cargo_client_yes_auth_backend() { + let _ = tracing_subscriber::fmt::fmt().with_test_writer().try_init(); + let listener = freighter::start_listening(freighter::cli::FreighterArgs { config: "../../configs/local.s3-based.yaml".into(), }) From bd8622494c994706f9b40d07e290edf63704b946 Mon Sep 17 00:00:00 2001 From: Kornel Date: Tue, 27 May 2025 14:34:47 +0100 Subject: [PATCH 7/7] Clippy --- crates/freighter-auth/src/cf_access.rs | 2 +- crates/freighter-auth/src/fs_backend.rs | 1 + crates/freighter-auth/src/yes_backend.rs | 1 + crates/freighter-server/tests/common/utils.rs | 2 +- crates/freighter/src/lib.rs | 4 ++-- crates/freighter/tests/cargo.rs | 19 +++++++++---------- 6 files changed, 15 insertions(+), 14 deletions(-) diff --git a/crates/freighter-auth/src/cf_access.rs b/crates/freighter-auth/src/cf_access.rs index de31fa2..d71b669 100644 --- a/crates/freighter-auth/src/cf_access.rs +++ b/crates/freighter-auth/src/cf_access.rs @@ -164,7 +164,7 @@ impl CfAccess { #[cfg(test)] #[tokio::test] -#[ignore] +#[ignore = "needs access token set up"] async fn cf_access_token_test() { // curl -sI -H "CF-Access-Client-Id: ….access" -H "CF-Access-Client-Secret: …" https://access.example.com | egrep -Eo 'CF_Authorization=[^;]+ let token = "…"; // Needs non-expired token ;( diff --git a/crates/freighter-auth/src/fs_backend.rs b/crates/freighter-auth/src/fs_backend.rs index fcbb3b1..28f2566 100644 --- a/crates/freighter-auth/src/fs_backend.rs +++ b/crates/freighter-auth/src/fs_backend.rs @@ -30,6 +30,7 @@ pub struct FsAuthProvider { } impl FsAuthProvider { + #[allow(clippy::needless_pass_by_value)] pub fn new(config: Config) -> AuthResult { std::fs::create_dir_all(&config.auth_path) .with_context(|| format!("Auth root at {}", config.auth_path.display())) diff --git a/crates/freighter-auth/src/yes_backend.rs b/crates/freighter-auth/src/yes_backend.rs index b297dd7..2618ebf 100644 --- a/crates/freighter-auth/src/yes_backend.rs +++ b/crates/freighter-auth/src/yes_backend.rs @@ -13,6 +13,7 @@ pub struct YesAuthProvider(()); impl YesAuthProvider { #[track_caller] + #[allow(clippy::needless_pass_by_value)] pub fn new(yes_config: Config) -> AuthResult { if !yes_config.auth_allow_full_access_without_any_checks { return Err(anyhow::anyhow!("enabled 'yes' auth without explicit opt-in").into()); diff --git a/crates/freighter-server/tests/common/utils.rs b/crates/freighter-server/tests/common/utils.rs index 900447e..1d4dc8b 100644 --- a/crates/freighter-server/tests/common/utils.rs +++ b/crates/freighter-server/tests/common/utils.rs @@ -59,7 +59,7 @@ pub fn generate_crate_payload( .to_string(); // https://github.com/rust-lang/cargo/blob/20df9e40a4d41dd08478549915588395e55efb4c/crates/crates-io/lib.rs#L259 - + let mut payload = Vec::new(); payload.extend_from_slice(&(json.len() as u32).to_le_bytes()); payload.extend_from_slice(json.as_bytes()); diff --git a/crates/freighter/src/lib.rs b/crates/freighter/src/lib.rs index bacad8e..c308db7 100644 --- a/crates/freighter/src/lib.rs +++ b/crates/freighter/src/lib.rs @@ -98,7 +98,7 @@ pub async fn start_listening( let listener = TcpListener::bind(addr).await?; - return Ok(async move { + Ok(async move { axum::serve(listener, router.into_make_service()) .with_graceful_shutdown(shutdown_signal()) .await @@ -106,7 +106,7 @@ pub async fn start_listening( tracing::info!("Completed graceful shutdown"); Ok(()) - }); + }) } // Based on: https://github.com/tokio-rs/axum/blob/main/examples/graceful-shutdown/src/main.rs diff --git a/crates/freighter/tests/cargo.rs b/crates/freighter/tests/cargo.rs index 6b81e41..70b1268 100644 --- a/crates/freighter/tests/cargo.rs +++ b/crates/freighter/tests/cargo.rs @@ -26,6 +26,7 @@ async fn cargo_client_yes_auth_backend() { .unwrap(); } +#[allow(dead_code)] fn test_cargo_publish() { let tempdir = tempfile::tempdir().unwrap(); let path = tempdir.path(); @@ -38,26 +39,24 @@ fn test_cargo_publish() { let cargo_config = cargo_dir.join("config.toml"); std::fs::write( &cargo_config, - format!( - r#" + r#" [registries.test_registry] index = "sparse+http://127.0.0.1:8080/index/" - "# - ), + "#, ) .unwrap(); let test_crate1 = path.join("test_crate1"); let src = test_crate1.join("src"); - std::fs::create_dir_all(&test_crate1.join("src")).unwrap(); + std::fs::create_dir_all(test_crate1.join("src")).unwrap(); std::fs::write( - &src.join("lib.rs"), + src.join("lib.rs"), format!("pub const ID_{random_id}: bool = true;"), ) .unwrap(); std::fs::write( - &test_crate1.join("Cargo.toml"), + test_crate1.join("Cargo.toml"), format!( r#" [package] @@ -82,15 +81,15 @@ publish = ["test_registry"] let test_crate2 = path.join("test_crate2"); let src = test_crate2.join("src"); - std::fs::create_dir_all(&test_crate2.join("src")).unwrap(); + std::fs::create_dir_all(test_crate2.join("src")).unwrap(); std::fs::write( - &src.join("lib.rs"), + src.join("lib.rs"), format!("pub use {test_crate1_name}::ID_{random_id};"), ) .unwrap(); std::fs::write( - &test_crate2.join("Cargo.toml"), + test_crate2.join("Cargo.toml"), format!( r#" [package]