diff --git a/src/admin/render_readmes.rs b/src/admin/render_readmes.rs index 273b6cad0cf..3eca842dcde 100644 --- a/src/admin/render_readmes.rs +++ b/src/admin/render_readmes.rs @@ -1,8 +1,7 @@ use crate::{ - config, db, + db, models::Version, schema::{crates, readme_renderings, versions}, - uploaders::Uploader, }; use anyhow::{anyhow, Context}; use std::{io::Read, path::Path, sync::Arc, thread}; @@ -40,7 +39,6 @@ pub struct Opts { } pub fn run(opts: Opts) -> anyhow::Result<()> { - let base_config = Arc::new(config::Base::from_environment()); let storage = Arc::new(Storage::from_environment()); let conn = &mut db::oneoff_connection().unwrap(); @@ -109,11 +107,10 @@ pub fn run(opts: Opts) -> anyhow::Result<()> { .context("Couldn't record rendering time")?; let client = client.clone(); - let base_config = base_config.clone(); let storage = storage.clone(); let handle = thread::spawn::<_, anyhow::Result<()>>(move || { println!("[{}-{}] Rendering README...", krate_name, version.num); - let readme = get_readme(base_config.uploader(), &client, &version, &krate_name)?; + let readme = get_readme(&storage, &client, &version, &krate_name)?; if !readme.is_empty() { let rt = tokio::runtime::Builder::new_current_thread() .enable_all() @@ -143,19 +140,14 @@ pub fn run(opts: Opts) -> anyhow::Result<()> { /// Renders the readme of an uploaded crate version. fn get_readme( - uploader: &Uploader, + storage: &Storage, client: &Client, version: &Version, krate_name: &str, ) -> anyhow::Result { let pkg_name = format!("{}-{}", krate_name, version.num); - let location = uploader.crate_location(krate_name, &version.num.to_string()); - - let location = match uploader { - Uploader::S3 { .. } => location, - Uploader::Local => format!("http://localhost:8888/{location}"), - }; + let location = storage.crate_location(krate_name, &version.num.to_string()); let mut extra_headers = header::HeaderMap::new(); extra_headers.insert( diff --git a/src/background_jobs.rs b/src/background_jobs.rs index 5214280300e..d2d9b7c3c5f 100644 --- a/src/background_jobs.rs +++ b/src/background_jobs.rs @@ -10,7 +10,6 @@ use crate::db::ConnectionPool; use crate::storage::Storage; use crate::swirl::errors::EnqueueError; use crate::swirl::PerformError; -use crate::uploaders::Uploader; use crate::worker; use crate::worker::cloudfront::CloudFront; use crate::worker::fastly::Fastly; @@ -294,7 +293,6 @@ pub struct RenderAndUploadReadmeJob { pub struct Environment { index: Arc>, - pub uploader: Uploader, http_client: AssertUnwindSafe, cloudfront: Option, fastly: Option, @@ -304,7 +302,6 @@ pub struct Environment { impl Environment { pub fn new( index: Repository, - uploader: Uploader, http_client: Client, cloudfront: Option, fastly: Option, @@ -312,7 +309,6 @@ impl Environment { ) -> Self { Self::new_shared( Arc::new(Mutex::new(index)), - uploader, http_client, cloudfront, fastly, @@ -322,7 +318,6 @@ impl Environment { pub fn new_shared( index: Arc>, - uploader: Uploader, http_client: Client, cloudfront: Option, fastly: Option, @@ -330,7 +325,6 @@ impl Environment { ) -> Self { Self { index, - uploader, http_client: AssertUnwindSafe(http_client), cloudfront, fastly, diff --git a/src/bin/background-worker.rs b/src/bin/background-worker.rs index 55f97ce29b6..57714d24263 100644 --- a/src/bin/background-worker.rs +++ b/src/bin/background-worker.rs @@ -40,7 +40,6 @@ fn main() { info!("Booting runner"); let config = config::Server::default(); - let uploader = config.base.uploader(); if config.db.are_all_read_only() { loop { @@ -83,14 +82,7 @@ fn main() { .build() .expect("Couldn't build client"); - let environment = Environment::new_shared( - repository, - uploader.clone(), - client, - cloudfront, - fastly, - storage, - ); + let environment = Environment::new_shared(repository, client, cloudfront, fastly, storage); let environment = Arc::new(Some(environment)); diff --git a/src/config/base.rs b/src/config/base.rs index f17e1f017ca..ae709b2da31 100644 --- a/src/config/base.rs +++ b/src/config/base.rs @@ -1,18 +1,11 @@ //! Base configuration options //! //! - `HEROKU`: Is this instance of crates_io:: currently running on Heroku. -//! - `S3_BUCKET`: The S3 bucket used to store crate files. If not present during development, -//! crates_io:: will fall back to a local uploader. -//! - `S3_REGION`: The region in which the bucket was created. Optional if US standard. -//! - `AWS_ACCESS_KEY`: The access key to interact with S3. -//! - `AWS_SECRET_KEY`: The secret key to interact with S3. -//! - `S3_CDN`: Optional CDN configuration for building public facing URLs. -use crate::{env, uploaders::Uploader, Env}; +use crate::Env; pub struct Base { pub env: Env, - pub uploader: Uploader, } impl Base { @@ -22,82 +15,6 @@ impl Base { _ => Env::Development, }; - let uploader = if env == Env::Production { - // `env` panics if these vars are not set, and in production for a primary instance, - // that's what we want since we don't want to be able to start the server if the - // server doesn't know where to upload crates. - Self::s3_panic_if_missing_keys() - } else if dotenvy::var("S3_BUCKET").is_ok() { - // If we've set the `S3_BUCKET` variable to any value, use all of the values - // for the related S3 environment variables and configure the app to upload to - // and read from S3 like production does. All values except for bucket are - // optional, like production read-only mirrors. - info!("Using S3 uploader"); - Self::s3_maybe_read_only() - } else { - // If we don't set the `S3_BUCKET` variable, we'll use a development-only - // uploader that makes it possible to run and publish to a locally-running - // crates.io instance without needing to set up an account and a bucket in S3. - info!("Using local uploader, crate files will be in the local_uploads directory"); - Uploader::Local - }; - - Self { env, uploader } - } - - pub fn uploader(&self) -> &Uploader { - &self.uploader - } - - fn s3_panic_if_missing_keys() -> Uploader { - let index_bucket = match dotenvy::var("S3_INDEX_BUCKET") { - Ok(name) => Some(Box::new(s3::Bucket::new( - name, - dotenvy::var("S3_INDEX_REGION") - .map_or_else(|_err| s3::Region::Default, s3::Region::Region), - env("AWS_ACCESS_KEY"), - env("AWS_SECRET_KEY"), - "https", - ))), - Err(_) => None, - }; - Uploader::S3 { - bucket: Box::new(s3::Bucket::new( - env("S3_BUCKET"), - dotenvy::var("S3_REGION") - .map_or_else(|_err| s3::Region::Default, s3::Region::Region), - env("AWS_ACCESS_KEY"), - env("AWS_SECRET_KEY"), - "https", - )), - index_bucket, - cdn: dotenvy::var("S3_CDN").ok(), - } - } - - fn s3_maybe_read_only() -> Uploader { - let index_bucket = match dotenvy::var("S3_INDEX_BUCKET") { - Ok(name) => Some(Box::new(s3::Bucket::new( - name, - dotenvy::var("S3_INDEX_REGION") - .map_or_else(|_err| s3::Region::Default, s3::Region::Region), - dotenvy::var("AWS_ACCESS_KEY").unwrap_or_default(), - dotenvy::var("AWS_SECRET_KEY").unwrap_or_default(), - "https", - ))), - Err(_) => None, - }; - Uploader::S3 { - bucket: Box::new(s3::Bucket::new( - env("S3_BUCKET"), - dotenvy::var("S3_REGION") - .map_or_else(|_err| s3::Region::Default, s3::Region::Region), - dotenvy::var("AWS_ACCESS_KEY").unwrap_or_default(), - dotenvy::var("AWS_SECRET_KEY").unwrap_or_default(), - "https", - )), - index_bucket, - cdn: dotenvy::var("S3_CDN").ok(), - } + Self { env } } } diff --git a/src/config/server.rs b/src/config/server.rs index 2a2ac2028e3..900c8a07be0 100644 --- a/src/config/server.rs +++ b/src/config/server.rs @@ -3,7 +3,7 @@ use ipnetwork::IpNetwork; use oauth2::{ClientId, ClientSecret}; use crate::publish_rate_limit::PublishRateLimit; -use crate::{env, env_optional, uploaders::Uploader, Env}; +use crate::{env, env_optional, Env}; use super::base::Base; use super::database_pools::DatabasePools; @@ -196,10 +196,6 @@ impl Server { pub fn env(&self) -> Env { self.base.env } - - pub fn uploader(&self) -> &Uploader { - self.base.uploader() - } } pub(crate) fn domain_name() -> String { diff --git a/src/controllers/krate/metadata.rs b/src/controllers/krate/metadata.rs index d482ddbc5cf..5c1f88df6a7 100644 --- a/src/controllers/krate/metadata.rs +++ b/src/controllers/krate/metadata.rs @@ -312,7 +312,7 @@ pub async fn readme( Path((crate_name, version)): Path<(String, String)>, req: Parts, ) -> Response { - let redirect_url = app.config.uploader().readme_location(&crate_name, &version); + let redirect_url = app.storage.readme_location(&crate_name, &version); if req.wants_json() { Json(json!({ "url": redirect_url })).into_response() } else { diff --git a/src/controllers/version/downloads.rs b/src/controllers/version/downloads.rs index e5fc5084359..925b575c631 100644 --- a/src/controllers/version/downloads.rs +++ b/src/controllers/version/downloads.rs @@ -125,7 +125,7 @@ pub async fn download( .await? }; - let redirect_url = app.config.uploader().crate_location(&crate_name, &version); + let redirect_url = app.storage.crate_location(&crate_name, &version); if wants_json { Ok(Json(json!({ "url": redirect_url })).into_response()) } else { diff --git a/src/lib.rs b/src/lib.rs index 6dcc29c59e4..2bdf36c37c1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -23,7 +23,7 @@ extern crate serde_json; #[macro_use] extern crate tracing; -pub use crate::{app::App, email::Emails, uploaders::Uploader}; +pub use crate::{app::App, email::Emails}; use std::str::FromStr; use std::sync::Arc; @@ -52,7 +52,6 @@ pub mod sql; pub mod ssh; pub mod swirl; mod test_util; -pub mod uploaders; pub mod util; pub mod worker; diff --git a/src/storage.rs b/src/storage.rs index 98db7138425..1dbc825d2f2 100644 --- a/src/storage.rs +++ b/src/storage.rs @@ -33,6 +33,7 @@ const CACHE_CONTROL_README: &str = "public,max-age=604800"; type StdPath = std::path::Path; #[derive(Debug)] +#[allow(clippy::large_enum_variant)] pub enum StorageConfig { S3 { default: S3Config, index: S3Config }, LocalFileSystem { path: PathBuf }, @@ -45,12 +46,14 @@ pub struct S3Config { region: Option, access_key: String, secret_key: SecretString, + cdn_prefix: Option, } impl StorageConfig { pub fn from_environment() -> Self { if let Ok(bucket) = dotenvy::var("S3_BUCKET") { let region = dotenvy::var("S3_REGION").ok(); + let cdn_prefix = dotenvy::var("S3_CDN").ok(); let index_bucket = env("S3_INDEX_BUCKET"); let index_region = dotenvy::var("S3_INDEX_REGION").ok(); @@ -63,6 +66,7 @@ impl StorageConfig { region, access_key: access_key.clone(), secret_key: secret_key.clone(), + cdn_prefix, }; let index = S3Config { @@ -70,6 +74,7 @@ impl StorageConfig { region: index_region, access_key, secret_key, + cdn_prefix: None, }; return Self::S3 { default, index }; @@ -90,6 +95,7 @@ pub struct Storage { crate_upload_store: Box, readme_upload_store: Box, db_dump_upload_store: Box, + cdn_prefix: String, index_store: Box, index_upload_store: Box, @@ -116,6 +122,14 @@ impl Storage { ClientOptions::default().with_default_content_type(CONTENT_TYPE_DB_DUMP); let db_dump_upload_store = build_s3(default, options); + let cdn_prefix = match default.cdn_prefix.as_ref() { + None => panic!("Missing S3_CDN environment variable"), + Some(cdn_prefix) if !cdn_prefix.starts_with("https://") => { + format!("https://{cdn_prefix}") + } + Some(cdn_prefix) => cdn_prefix.clone(), + }; + let options = ClientOptions::default(); let index_store = build_s3(index, options); @@ -127,6 +141,7 @@ impl Storage { crate_upload_store: Box::new(crate_upload_store), readme_upload_store: Box::new(readme_upload_store), db_dump_upload_store: Box::new(db_dump_upload_store), + cdn_prefix, index_store: Box::new(index_store), index_upload_store: Box::new(index_upload_store), } @@ -157,6 +172,7 @@ impl Storage { crate_upload_store: Box::new(store.clone()), readme_upload_store: Box::new(store.clone()), db_dump_upload_store: Box::new(store), + cdn_prefix: "/".into(), index_store: Box::new(index_store.clone()), index_upload_store: Box::new(index_store), } @@ -171,6 +187,7 @@ impl Storage { crate_upload_store: Box::new(store.clone()), readme_upload_store: Box::new(store.clone()), db_dump_upload_store: Box::new(store.clone()), + cdn_prefix: "/".into(), index_store: Box::new(PrefixStore::new(store.clone(), "index")), index_upload_store: Box::new(PrefixStore::new(store, "index")), } @@ -178,6 +195,20 @@ impl Storage { } } + /// Returns the URL of an uploaded crate's version archive. + /// + /// The function doesn't check for the existence of the file. + pub fn crate_location(&self, name: &str, version: &str) -> String { + format!("{}{}", self.cdn_prefix, crate_file_path(name, version)).replace('+', "%2B") + } + + /// Returns the URL of an uploaded crate's version readme. + /// + /// The function doesn't check for the existence of the file. + pub fn readme_location(&self, name: &str, version: &str) -> String { + format!("{}{}", self.cdn_prefix, readme_path(name, version)).replace('+', "%2B") + } + #[instrument(skip(self))] pub async fn delete_all_crate_files(&self, name: &str) -> Result<()> { let prefix = format!("{PREFIX_CRATES}/{name}").into(); @@ -328,6 +359,35 @@ mod tests { .collect() } + #[test] + fn locations() { + let storage = Storage::from_config(&StorageConfig::InMemory); + + let crate_tests = vec![ + ("foo", "1.2.3", "/crates/foo/foo-1.2.3.crate"), + ( + "some-long-crate-name", + "42.0.5-beta.1+foo", + "/crates/some-long-crate-name/some-long-crate-name-42.0.5-beta.1%2Bfoo.crate", + ), + ]; + for (name, version, expected) in crate_tests { + assert_eq!(storage.crate_location(name, version), expected); + } + + let readme_tests = vec![ + ("foo", "1.2.3", "/readmes/foo/foo-1.2.3.html"), + ( + "some-long-crate-name", + "42.0.5-beta.1+foo", + "/readmes/some-long-crate-name/some-long-crate-name-42.0.5-beta.1%2Bfoo.html", + ), + ]; + for (name, version, expected) in readme_tests { + assert_eq!(storage.readme_location(name, version), expected); + } + } + #[tokio::test] async fn delete_all_crate_files() { let storage = prepare().await; diff --git a/src/tests/util/test_app.rs b/src/tests/util/test_app.rs index 3d27d9137e4..1f4c18f5ee2 100644 --- a/src/tests/util/test_app.rs +++ b/src/tests/util/test_app.rs @@ -3,7 +3,7 @@ use crate::record; use crate::util::{chaosproxy::ChaosProxy, fresh_schema::FreshSchema}; use crates_io::config::{self, BalanceCapacityConfig, Base, DatabasePools, DbPoolConfig}; use crates_io::storage::StorageConfig; -use crates_io::{background_jobs::Environment, env, App, Emails, Env, Uploader}; +use crates_io::{background_jobs::Environment, env, App, Emails, Env}; use crates_io_index::testing::UpstreamIndex; use crates_io_index::{Credentials, Repository as WorkerRepository, RepositoryConfig}; use std::{rc::Rc, sync::Arc, time::Duration}; @@ -259,7 +259,6 @@ impl TestAppBuilder { let index = WorkerRepository::open(&repository_config).expect("Could not clone index"); let environment = Environment::new( index, - app.config.uploader().clone(), app.http_client().clone(), None, None, @@ -356,32 +355,7 @@ impl TestAppBuilder { } fn simple_config() -> config::Server { - let uploader = Uploader::S3 { - bucket: Box::new(s3::Bucket::new( - dotenvy::var("TEST_S3_BUCKET").unwrap_or_else(|_err| "crates-test".into()), - parse_test_region(dotenvy::var("TEST_S3_REGION").ok()), - dotenvy::var("TEST_AWS_ACCESS_KEY").unwrap_or_default(), - dotenvy::var("TEST_AWS_SECRET_KEY").unwrap_or_default(), - // When testing we route all API traffic over HTTP so we can - // sniff/record it, but everywhere else we use https - "http", - )), - index_bucket: Some(Box::new(s3::Bucket::new( - dotenvy::var("TEST_S3_INDEX_BUCKET").unwrap_or_else(|_err| "crates-index-test".into()), - parse_test_region(dotenvy::var("TEST_S3_INDEX_REGION").ok()), - dotenvy::var("TEST_AWS_ACCESS_KEY").unwrap_or_default(), - dotenvy::var("TEST_AWS_SECRET_KEY").unwrap_or_default(), - // When testing we route all API traffic over HTTP so we can - // sniff/record it, but everywhere else we use https - "http", - ))), - cdn: None, - }; - - let base = Base { - env: Env::Test, - uploader, - }; + let base = Base { env: Env::Test }; let db = DatabasePools { primary: DbPoolConfig { diff --git a/src/uploaders.rs b/src/uploaders.rs deleted file mode 100644 index 331510c1afc..00000000000 --- a/src/uploaders.rs +++ /dev/null @@ -1,70 +0,0 @@ -#[derive(Clone, Debug)] -pub enum Uploader { - /// For production usage, uploads and redirects to s3. - /// For test usage with `TestApp::with_proxy()`, the recording proxy is used. - S3 { - bucket: Box, - index_bucket: Option>, - cdn: Option, - }, - - /// For development usage only: "uploads" crate files to `dist` and serves them - /// from there as well to enable local publishing and download - Local, -} - -impl Uploader { - /// Returns the URL of an uploaded crate's version archive. - /// - /// The function doesn't check for the existence of the file. - pub fn crate_location(&self, crate_name: &str, version: &str) -> String { - let version = version.replace('+', "%2B"); - - match *self { - Uploader::S3 { - ref bucket, - ref cdn, - .. - } => { - let path = Uploader::crate_path(crate_name, &version); - match *cdn { - Some(ref host) => format!("https://{host}/{path}"), - None => bucket.url(&path).unwrap(), - } - } - Uploader::Local => format!("/{}", Uploader::crate_path(crate_name, &version)), - } - } - - /// Returns the URL of an uploaded crate's version readme. - /// - /// The function doesn't check for the existence of the file. - pub fn readme_location(&self, crate_name: &str, version: &str) -> String { - let version = version.replace('+', "%2B"); - - match *self { - Uploader::S3 { - ref bucket, - ref cdn, - .. - } => { - let path = Uploader::readme_path(crate_name, &version); - match *cdn { - Some(ref host) => format!("https://{host}/{path}"), - None => bucket.url(&path).unwrap(), - } - } - Uploader::Local => format!("/{}", Uploader::readme_path(crate_name, &version)), - } - } - - /// Returns the internal path of an uploaded crate's version archive. - pub fn crate_path(name: &str, version: &str) -> String { - format!("crates/{name}/{name}-{version}.crate") - } - - /// Returns the internal path of an uploaded crate's version readme. - pub fn readme_path(name: &str, version: &str) -> String { - format!("readmes/{name}/{name}-{version}.html") - } -}