Support minio (#288)

S3 has two endpoints, a “path-based” endpoint and “virtual-hosted-style”
endpoints. ([read
more](https://medium.com/@e.osama85/the-difference-between-the-amazon-s3-path-style-urls-and-virtual-hosted-style-urls-4fafd5eca4db))

Virtual hosted style uses a different hostname for each bucket, while
path-style uses one endpoint and puts the bucket name in the path.
Path-style is easier to set up when using an S3-compatible API like
minio, because you don't have to deal with DNS certs etc.

Previously, we only used path-style if the URL was set to `localhost`,
which was basically only useful for our tests that ran `minio` locally.
This change allows the use of path-style URLs by setting the
`AWS_S3_USE_PATH_STYLE` env variable. It also supports the old
`localhost` behavior, but warns and suggests using
`AWS_S3_USE_PATH_STYLE`.

## `bucket_prefix` bug fix

This also fixes a bug where `Some("")` was being used as a
`bucket_prefix`. When a bucket prefix is a `Some`, we prepend it to the
path separated by "/", as in "{bucket_prefix}/{path}". With an empty
`bucket_prefix`, the resulting path is an "absolute-path" (since it
begins with a slash) instead of a "relative path" as the rest of the
code expects.

In particular, `Url::join` behaves very differently when passed an
"absolute path" than a relative one: with an absolute path, the new path
_replaces_ the whole other old one.

So what was happening was that when we had a `bucket_prefix` of `/`, if
the bucket name was in the path (as in path-style URLs), the bucket name
was inadvertently removed from the path.

The fix is as simple as ensuring that `Some("")` is converted to `None`,
as in:

```rust
let bucket_prefix = (!bucket_prefix.is_empty()).then(|| bucket_prefix);
```

Author: paulgb

crates/y-sweet-core/src/store/s3.rs

@@ -18,6 +18,9 @@ pub struct S3Config {
     pub bucket: String,
     pub region: String,
     pub bucket_prefix: Option<String>,
+
+    // Use old path-style URLs, needed to support some S3-compatible APIs (including some minio setups)
+    pub path_style: bool,
 }
 
 const PRESIGNED_URL_DURATION: Duration = Duration::from_secs(60 * 60);
@@ -38,14 +41,18 @@ impl S3Store {
             Credentials::new(config.key, config.secret)
         };
         let endpoint: Url = config.endpoint.parse().expect("endpoint is a valid url");
-        let path_style =
-            // if endpoint is localhost then bucket url must be of form http://localhost:<port>/<bucket>
-            // instead of <method>:://<bucket>.<endpoint>
-            if endpoint.host_str().expect("endpoint Url should have host") == "localhost" {
-                rusty_s3::UrlStyle::Path
-            } else {
-                rusty_s3::UrlStyle::VirtualHost
-            };
+
+        let path_style = if config.path_style {
+            rusty_s3::UrlStyle::Path
+        } else if endpoint.host_str() == Some("localhost") {
+            // Since this was the old behavior before we added AWS_S3_USE_PATH_STYLE,
+            // we continue to support it, but complain a bit.
+            tracing::warn!("Inferring path-style URLs for localhost for backwards-compatibility. This behavior may change in the future. Set AWS_S3_USE_PATH_STYLE=true to ensure that path-style URLs are used.");
+            rusty_s3::UrlStyle::Path
+        } else {
+            rusty_s3::UrlStyle::VirtualHost
+        };
+
         let bucket = Bucket::new(endpoint, path_style, config.bucket, config.region)
             .expect("Url has a valid scheme and host");
         let client = Client::new();

crates/y-sweet-worker/Cargo.lock

@@ -88,6 +88,7 @@ fn parse_s3_config(env: &Env) -> anyhow::Result<S3Config> {
             .map_err(|_| anyhow::anyhow!("S3_BUCKET_NAME env var not supplied"))?
             .to_string(),
         bucket_prefix: env.var(S3_BUCKET_PREFIX).ok().map(|t| t.to_string()),
+        path_style: false,
     })
 }
 

crates/y-sweet-worker/src/config.rs

@@ -89,7 +89,26 @@ const S3_SECRET_ACCESS_KEY: &str = "AWS_SECRET_ACCESS_KEY";
 const S3_SESSION_TOKEN: &str = "AWS_SESSION_TOKEN";
 const S3_REGION: &str = "AWS_REGION";
 const S3_ENDPOINT: &str = "AWS_ENDPOINT_URL_S3";
-fn parse_s3_config_from_env_and_args(bucket: String, prefix: String) -> anyhow::Result<S3Config> {
+const S3_USE_PATH_STYLE: &str = "AWS_S3_USE_PATH_STYLE";
+fn parse_s3_config_from_env_and_args(
+    bucket: String,
+    prefix: Option<String>,
+) -> anyhow::Result<S3Config> {
+    let use_path_style = env::var(S3_USE_PATH_STYLE).ok();
+    let path_style = if let Some(use_path_style) = use_path_style {
+        if use_path_style.to_lowercase() == "true" {
+            true
+        } else if use_path_style.to_lowercase() == "false" || use_path_style.is_empty() {
+            false
+        } else {
+            anyhow::bail!(
+                "If AWS_S3_USE_PATH_STYLE is set, it must be either \"true\" or \"false\""
+            )
+        }
+    } else {
+        false
+    };
+
     Ok(S3Config {
         key: env::var(S3_ACCESS_KEY_ID)
             .map_err(|_| anyhow::anyhow!("{} env var not supplied", S3_ACCESS_KEY_ID))?,
@@ -104,7 +123,9 @@ fn parse_s3_config_from_env_and_args(bucket: String, prefix: String) -> anyhow::
             .map_err(|_| anyhow::anyhow!("{} env var not supplied", S3_SECRET_ACCESS_KEY))?,
         token: env::var(S3_SESSION_TOKEN).ok(),
         bucket,
-        bucket_prefix: Some(prefix),
+        bucket_prefix: prefix,
+        // If the endpoint is overridden, we assume that the user wants path-style URLs.
+        path_style,
     })
 }
 
@@ -116,6 +137,7 @@ fn get_store_from_opts(store_path: &str) -> Result<Box<dyn Store>> {
             .ok_or_else(|| anyhow::anyhow!("Invalid S3 URL"))?
             .to_owned();
         let bucket_prefix = url.path().trim_start_matches('/').to_owned();
+        let bucket_prefix = (!bucket_prefix.is_empty()).then(|| bucket_prefix); // "" => None
         let config = parse_s3_config_from_env_and_args(bucket, bucket_prefix)?;
         let store = S3Store::new(config);
         Ok(Box::new(store))
@@ -239,6 +261,7 @@ async fn main() -> Result<()> {
 
             let store = match (bucket, prefix) {
                 (Some(bucket), Some(prefix)) => {
+                    let prefix = (!prefix.is_empty()).then(|| prefix);
                     let s3_config = parse_s3_config_from_env_and_args(bucket, prefix)
                         .context("Failed to parse S3 configuration")?;
                     let store = S3Store::new(s3_config);