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 cross region bucket access #23652

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

zhaner08
Copy link
Contributor

@zhaner08 zhaner08 commented Oct 2, 2024

Description

Support s3 cross region bucket access

Additional context and related issues

In the legacy filesystem, this is not configurable but when region and endpoint is not set, us-east-1 will be set (which is also the default when setting aws-global) and cross region bucket access is allowed. The sdk v2 does not have this option earlier and now it supports it.

With this PR, we want to make this is configurable, while also set region and cross region access when no region nor endpoint is defined. When region is defined, and endpoint is not, then we will look at this config to determine if cross region access is allowed.

We need this config to guard that in certain regions, no cross region access is allowed, while in some other regions, cross region access can be done

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( X) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

## Section
* Fix some things. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label Oct 2, 2024
@zhaner08 zhaner08 requested a review from electrum October 2, 2024 21:24
@zhaner08 zhaner08 self-assigned this Oct 4, 2024
@@ -115,6 +115,7 @@ public static RetryStrategy getRetryStrategy(RetryMode retryMode)
private RetryMode retryMode = RetryMode.LEGACY;
private int maxErrorRetries = 10;
private boolean supportsExclusiveCreate = true;
private boolean crossRegionAccessEnabled;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add documentation in file-system-s3.md

awsEndpoint.ifPresent(s3::endpointOverride);
if (isCrossRegionAccessEnabled && awsEndpoint.isEmpty()) {
if (awsRegion.isEmpty()) {
s3.region(Region.US_EAST_1);
Copy link
Member

@electrum electrum Oct 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why hard-code to a specific region?

If we remove this code, what happens?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants