Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression: STORAGE_<LOCATION>_REGION should default to 'us-east-1' for s3 driver #17141

Closed
helge000 opened this issue Jan 13, 2023 · 10 comments
Closed
Assignees

Comments

@helge000
Copy link

Describe the Bug

When running Directus with a connected object store for files connections fails with error when STORAGE_<LOCATION>_REGION is unset.

This is a regression since this parameter was not required in the past (def. Directus 9.20.4)

To Reproduce

Location unset

  1. start a docker directus instance with configured object store driver s3
  2. Do not set STORAGE_<LOCATION>_REGION
  3. Upload a new file
  4. Observe error

Location set to any value except auto

  1. start a docker directus instance with configured object store driver s3
  2. set STORAGE_<LOCATION>_REGION to foo
  3. Upload a new file
  4. Observe error

Errors Shown

STORAGE__REGION unset

WARN: Couldn't save file 4770c227-eba2-48de-be8c-5f7881b394ec.png
[09:38:52.899] WARN: Region is missing
    err: {
       "type": "Error",
       "message": "Region is missing",
[...]

STORAGE__REGION set to foo

WARN: Couldn't save file 4770c227-eba2-48de-be8c-5f7881b394ec.png
[09:38:52.899] ERROR: The region name 'foo' is not valid. Must be 'auto'
[...]

What version of Directus are you using?

9.22.4

What version of Node.js are you using?

official docker image

What database are you using?

Pg 12

What browser are you using?

n/a

How are you deploying Directus?

Docker

@github-actions
Copy link

Linear: ENG-484

@rijkvanzanten
Copy link
Member

What env vars did you configure? Does endpoint exist? Are you connecting to S3 or a S3 compatible service?

@helge000
Copy link
Author

The following vars are set:

STORAGE_LOCATIONS: r2
STORAGE_R2_DRIVER: s3
STORAGE_R2_REGION: auto
STORAGE_R2_BUCKET: ${DIRECTUS_R2_BUCKET}
STORAGE_R2_ENDPOINT: ${DIRECTUS_R2_ENDPOINT}
STORAGE_R2_KEY: ${DIRECTUS_R2_KEY}
STORAGE_R2_SECRET: ${DIRECTUS_R2_SECRET}

As you might guess from the first one I am using R2 in S3 compat mode (I use BUCKET for convenience).

@rijkvanzanten
Copy link
Member

@helge000 It looks like it might be something R2 relies on only, as I haven't been able to find any documentation or reference to region: auto outside of the Cloudflare docs just now. From some light testing just now, it looks like minio for example happily works without providing a region 🤔

Maybe Cloudflare requires the region flag to be configured, and for some mysterious way that was preconfigured automatically in aws-sdk@v2 whereas it needs to be configured in client-s3?

@helge000
Copy link
Author

Right, I found the errormsg about the auto region weirdly specific.

That said, I have several Directus instances running with R2 baked storage the same. Only the one I spun up yesterday (v9.22.4) needed the region to be set - all others are working flawless without (the most recent being v9.20.4). I can test this further on Monday if there is any point in doing so.

@rijkvanzanten
Copy link
Member

@helge000 We upgraded from the older aws-sdk to the current stable release of @aws-sdk/client-s3 in between those versions, so that explains the difference in behavior. The thing that does confuse me however is that the previous aws-sdk apparently just worked out of the box without specifying that region?

@rijkvanzanten
Copy link
Member

I'm also not entirely sure if it's the AWS SDK throwing that error, or Cloudflare. I'm not able to get that error message on minio or S3 usage for example 🤔

@helge000
Copy link
Author

helge000 commented Jan 16, 2023

@rijkvanzanten , I did a little digging and I think this is an upstream issue in client-s3 - specifically between v2 and v3. In v2, the region defaulted to us-east-1 when empty. v3 changes this behaviour.

The agreed upon workaround seems to be to set the region default to us-east-1. I modified my issue to reflect this.

I can confirm us-east-1 works with R2 no problem.

@helge000 helge000 changed the title Regression: STORAGE_<LOCATION>_REGION should default to 'auto' for s3 driver Regression: STORAGE_<LOCATION>_REGION should default to 'us-east-1' for s3 driver Jan 16, 2023
@rijkvanzanten
Copy link
Member

Ahh that makes sense. Thanks for investigating that @helge000!

@rijkvanzanten
Copy link
Member

rijkvanzanten commented Jul 12, 2023

On a fresh look now, I don't think defaulting to us-east-1 is the appropriate fix here. AWS SDK v2 supported global buckets which would be accessed through the default us-east-1, but AWS SDK v3 doesn't support that (as per the ticket you shared). Setting a default would make it work without a configured region for Cloudflare specifically, but would most likely cause hard to catch bugs when a bucket is being accessed from the wrong region. In hindsight, we'll treat this as a breaking change between 9.20 and 9.22.0. I've updated the release notes of the offending version with a breaking change warning.

@rijkvanzanten rijkvanzanten closed this as not planned Won't fix, can't repro, duplicate, stale Jul 12, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

2 participants