storage: move S3_REGION and S3_ENDPOINT to Config

Author: pietroalbini

src/config.rs

@@ -1,5 +1,6 @@
 use crate::storage::StorageBackendKind;
 use failure::{bail, format_err, Error, Fail, ResultExt};
+use rusoto_core::Region;
 use std::env::VarError;
 use std::path::PathBuf;
 use std::str::FromStr;
@@ -22,6 +23,8 @@ pub struct Config {
 
     // S3 params
     pub(crate) s3_bucket: String,
+    pub(crate) s3_region: Region,
+    pub(crate) s3_endpoint: Option<String>,
     #[cfg(test)]
     pub(crate) s3_bucket_is_temporary: bool,
 
@@ -55,6 +58,8 @@ impl Config {
             storage_backend: env("DOCSRS_STORAGE_BACKEND", StorageBackendKind::Database)?,
 
             s3_bucket: env("DOCSRS_S3_BUCKET", "rust-docs-rs".to_string())?,
+            s3_region: env("S3_REGION", Region::UsWest1)?,
+            s3_endpoint: maybe_env("S3_ENDPOINT")?,
             // DO NOT CONFIGURE THIS THROUGH AN ENVIRONMENT VARIABLE!
             // Accidentally turning this on outside of the test suite might cause data loss in the
             // production environment.

src/storage/mod.rs

@@ -105,11 +105,9 @@ impl Storage {
                 StorageBackendKind::Database => {
                     StorageBackend::Database(DatabaseBackend::new(pool, metrics))
                 }
-                StorageBackendKind::S3 => StorageBackend::S3(Box::new(S3Backend::new(
-                    s3::s3_client().unwrap(),
-                    metrics,
-                    config,
-                )?)),
+                StorageBackendKind::S3 => {
+                    StorageBackend::S3(Box::new(S3Backend::new(metrics, config)?))
+                }
             },
         })
     }

src/storage/s3.rs

@@ -6,7 +6,6 @@ use futures_util::{
     future::TryFutureExt,
     stream::{FuturesUnordered, StreamExt},
 };
-use log::warn;
 use rusoto_core::{region::Region, RusotoError};
 use rusoto_credential::DefaultCredentialsProvider;
 use rusoto_s3::{
@@ -26,13 +25,23 @@ pub(super) struct S3Backend {
 }
 
 impl S3Backend {
-    pub(super) fn new(
-        client: S3Client,
-        metrics: Arc<Metrics>,
-        config: &Config,
-    ) -> Result<Self, Error> {
+    pub(super) fn new(metrics: Arc<Metrics>, config: &Config) -> Result<Self, Error> {
         let runtime = Runtime::new()?;
 
+        // Connect to S3
+        let client = S3Client::new_with(
+            rusoto_core::request::HttpClient::new()?,
+            DefaultCredentialsProvider::new()?,
+            config
+                .s3_endpoint
+                .as_deref()
+                .map(|endpoint| Region::Custom {
+                    name: config.s3_region.name().to_string(),
+                    endpoint: endpoint.to_string(),
+                })
+                .unwrap_or(config.s3_region.clone()),
+        );
+
         #[cfg(test)]
         {
             // Create the temporary S3 bucket during tests.
@@ -275,34 +284,6 @@ fn parse_timespec(mut raw: &str) -> Result<DateTime<Utc>, Error> {
     ))
 }
 
-pub(super) fn s3_client() -> Option<S3Client> {
-    // If AWS keys aren't configured, then presume we should use the DB exclusively
-    // for file storage.
-    if std::env::var_os("AWS_ACCESS_KEY_ID").is_none() && std::env::var_os("FORCE_S3").is_none() {
-        return None;
-    }
-
-    let creds = match DefaultCredentialsProvider::new() {
-        Ok(creds) => creds,
-        Err(err) => {
-            warn!("failed to retrieve AWS credentials: {}", err);
-            return None;
-        }
-    };
-
-    Some(S3Client::new_with(
-        rusoto_core::request::HttpClient::new().unwrap(),
-        creds,
-        std::env::var("S3_ENDPOINT")
-            .ok()
-            .map(|e| Region::Custom {
-                name: std::env::var("S3_REGION").unwrap_or_else(|_| "us-west-1".to_owned()),
-                endpoint: e,
-            })
-            .unwrap_or(Region::UsWest1),
-    ))
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;