wojtekmach / req_s3

26 stars 8 forks source link

Support `AWS_ENDPOINT_URL_S3` #3

Closed wojtekmach closed 2 months ago

wojtekmach commented 3 months ago

At the moment, s3://bucket is always turned into https://bucket.s3.amazonaws.com. And when the URL path is empty (i.e. URL is s3://bucket but not s3://bucket/key), we automatically decode the <ListBucketResult> XML response.

I'd like to support Tigris as well as other S3-compatible services out of the box. The standard way is to have an AWS_ENDPOINT_URL_S3 environment variable which Fly/Tigris already sets.

Option 1

ReqS3 would register a :aws_endpoint_url_s3 or similar option and so we have this:

req =
  Req.new()
  |> ReqS3.attach(
    aws_sigv4: [
      access_key_id: System.fetch_env!("AWS_ACCESS_KEY_ID"),
      secret_access_key: System.fetch_env!("AWS_SECRET_ACCESS_KEY")
    ],
    aws_endpoint_url_s3: System.fetch_env!("AWS_ENDPOINT_URL_S3")
  )

Req.get!(req, url: "s3://#{bucket}")

I think it'd be maybe a little weird that we have :aws_sigv4 (coming from Req proper) and another :aws_ top-level option but they do serve different purposes. We could support aws_sigv4: [endpoint_url_s3: ...] but it'd be weird because the endpoint url is not used for signature generation.

Option 2

AWS_ENDPOINT_URL_S3, AWS_ACCESS_KEY_ID, and AWS_SECRET_ACCESS_KEY are automatically picked up by ReqS3 on s3:// schema (not by Req proper):

req = Req.new() |> ReqS3.attach()
Req.get!(req, url: "s3://#{bucket}")

You could still manually configure everything just like in Option 1.

Furthermore, we'd use these env vars in pre-signing functions:

  ReqS3.presign_url(
-   access_key_id: System.fetch_env!("AWS_ACCESS_KEY_ID"),
-   secret_access_key: System.fetch_env!("AWS_SECRET_ACCESS_KEY"),
-   url: "https://fly.storage.tigris.dev/#{bucket}/#{key}"
+   bucket: bucket,
+   key: key
  )

I'm leaning towards this option 2. Thoughts?

jwbaldwin commented 3 months ago

Was looking for this exact configuration option. My drive by thought is that option 2 feels like what I expect from this library: a more done-for-you approach (while still allowing manual configuration).

wojtekmach commented 2 months ago

Done!