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

support s3 global client #1807

Closed
AllanZhengYP opened this issue Dec 17, 2020 · 7 comments
Closed

support s3 global client #1807

AllanZhengYP opened this issue Dec 17, 2020 · 7 comments
Labels
feature-request New feature or enhancement. May require GitHub community feedback. p1 This is a high priority issue queued This issues is on the AWS team's backlog

Comments

@AllanZhengYP
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Ported from comment: #1802 (comment):
n v2, we implemented a lot of hacky logic to make the S3 client behaves like a global client, but it may confuse the users, espacially those having knowledge of other AWS tools. So we deliberately removed the follow-the-redirect feature in v3. But it may bring some confusions when users see 301 response from S3. Other AWS SDKs support global client that allow users to make request to other regions.

Describe the solution you'd like
The implementation will be similar to that in v2 s3 client. We can either ship it in a higher level library, or an SDK plugin, or a client config.

@github-actions
Copy link

Greetings! We’re closing this issue because it has been open a long time and hasn’t been updated in a while and may not be getting the attention it deserves. We encourage you to check if this is still an issue in the latest release and if you find that this is still a problem, please feel free to comment or open a new issue.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Dec 18, 2021
@trivikr trivikr reopened this Dec 23, 2021
@benji1298
Copy link

This issue is currently blocking us from upgrading to v3 of the SDK.

We access many S3 buckets from many different regions. In our v2 code, we are able to set up a single S3 client without specifying a region and access objects via Bucket & Key. It does not matter where those buckets live - the v2 S3 client handles any redirects. Now, in v3, we are getting 301 errors.

It will not be feasible to go through every bucket, identify the region, and create it's own S3 client with a hard-coded region. Are there any workarounds available to create a global client?

@ahnv
Copy link

ahnv commented Jul 12, 2022

@benji1298 We had a similar issue while moving from V2 SDK to V3 SDK, so we came with a workaround that leverages those 301 redirects and saves it in Redis for later use

export const getS3RegionalClient = async (s3Config: S3ClientConfig, params: GetObjectCommandInput) => {
  const { Bucket } = params

  if (s3Config.endpoint) {
    // Return S3 Client with us-east-1 region
    // See https://github.com/aws/aws-sdk-js-v3/issues/1845
  }

  let region = getRegionFromRedis(Bucket)
  if (!region) {
    const options: S3ClientConfig = { region: "us-east-1" }
    if (s3Config.credentials) {
      options.credentials = s3Config.credentials
    }
    const client = new S3(options)
    try {
      await client.getObject(params)
      region = "us-east-1"
    } catch (err: any) {
      // Us-East-1 Will return PermanentRedirect, irrespective of the fact if listbucket permission is given or not
      // as logically it seems that s3 keeps permissions at the region of buckets rather than keeping it globally across all data centers
      if (err.Code === "PermanentRedirect") {
        if (err.Endpoint) {
          const regionSlug: string = err.Endpoint
          const matches = regionSlug.match(new RegExp(S3RegionList.join("|")))
          if (matches?.[0]) {
            region = matches[0]
          }
        }
      }

      // without listBucket permission, the actual bucket will return the accessdenied permission
      if (err.name === "NoSuchKey" || err.name === "AccessDenied") {
        region = "us-east-1"
      }

      if (!region) {
        try {
          const { LocationConstraint } = await client.getBucketLocation({ Bucket })
          region = LocationConstraint || "us-east-1"
        } catch (locationError: any) {
          console.log(locationError)
        }

        console.log(err)
        throw Error("Unknown S3 Region")
      }
    }
    await saveRegionInRedis(Bucket, region)
}

@RanVaknin RanVaknin added p2 This is a standard priority issue p1 This is a high priority issue and removed p2 This is a standard priority issue labels Mar 7, 2023
@markandrus
Copy link

markandrus commented Apr 11, 2023

Here's another workaround that patches the S3Client's send method. It's not perfectly type safe and only supports the Promise interface, but it's something. It will probably break if you use the same S3Client to concurrently access buckets in multiple regions, which is why it would be nicer to handle this in the SDK natively.

function handlePermanentRedirect (client: S3Client): void {
  const originalSend: any = client.send

  client.send = async function updatedSend (...args: any[]): Promise<any> {
    try {
      return await originalSend.apply(client, args)
    } catch (error) {
      if (error instanceof S3ServiceException && (error.name === 'PermanentRedirect' || error.$response?.statusCode === 301)) {
        const region = error.$response?.headers['x-amz-bucket-region']
        if (region !== undefined) {
          (client.config.region as any) = region
          return await originalSend.apply(client, args)
        }
      }
      throw error
    }
  }
}

UPDATE: Something about this didn't play nicely with our minification process. We now maintain a lazily constructed Map from AWS region to S3Client. It's slightly safer. The logic for catching the PermanentRedirect is the same.

@yenfryherrerafeliz yenfryherrerafeliz added the queued This issues is on the AWS team's backlog label May 3, 2023
@yenfryherrerafeliz
Copy link
Contributor

Related issue here.

@mshjri
Copy link

mshjri commented May 10, 2023

if you came here while trying to copy an object from one region to another, set the client's region as the destination bucket's region.

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request New feature or enhancement. May require GitHub community feedback. p1 This is a high priority issue queued This issues is on the AWS team's backlog
Projects
None yet
Development

No branches or pull requests

9 participants