[
https://issues.apache.org/jira/browse/HADOOP-19740?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18038995#comment-18038995
]
ASF GitHub Bot commented on HADOOP-19740:
-----------------------------------------
mukund-thakur commented on code in PR #8058:
URL: https://github.com/apache/hadoop/pull/8058#discussion_r2535731487
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/DefaultS3ClientFactory.java:
##########
@@ -319,162 +295,63 @@ protected ClientOverrideConfiguration.Builder
createClientOverrideConfiguration(
* <li> S3 cross region is enabled by default irrespective of region or
endpoint
* is set or not.</li>
* </ol>
- *
* @param builder S3 client builder.
* @param parameters parameter object
- * @param conf conf configuration object
+ * @param conf conf configuration object
* @param <BuilderT> S3 client builder type
* @param <ClientT> S3 client type
+ * @return how the region was resolved.
* @throws IllegalArgumentException if endpoint is set when FIPS is enabled.
*/
- private <BuilderT extends S3BaseClientBuilder<BuilderT, ClientT>, ClientT>
void configureEndpointAndRegion(
- BuilderT builder, S3ClientCreationParameters parameters, Configuration
conf) {
- final String endpointStr = parameters.getEndpoint();
- final URI endpoint = getS3Endpoint(endpointStr, conf);
-
- final String configuredRegion = parameters.getRegion();
- Region region = null;
- String origin = "";
-
- // If the region was configured, set it.
- if (configuredRegion != null && !configuredRegion.isEmpty()) {
- origin = AWS_REGION;
- region = Region.of(configuredRegion);
- }
+ private <BuilderT extends S3BaseClientBuilder<BuilderT, ClientT>, ClientT>
RegionResolution.Resolution configureEndpointAndRegion(
+ BuilderT builder, S3ClientCreationParameters parameters, Configuration
conf) throws IOException {
- // FIPs? Log it, then reject any attempt to set an endpoint
- final boolean fipsEnabled = parameters.isFipsEnabled();
- if (fipsEnabled) {
- LOG.debug("Enabling FIPS mode");
- }
- // always setting it guarantees the value is non-null,
- // which tests expect.
- builder.fipsEnabled(fipsEnabled);
-
- if (endpoint != null) {
- boolean endpointEndsWithCentral =
- endpointStr.endsWith(CENTRAL_ENDPOINT);
- checkArgument(!fipsEnabled || endpointEndsWithCentral, "%s : %s",
- ERROR_ENDPOINT_WITH_FIPS,
- endpoint);
-
- // No region was configured,
- // determine the region from the endpoint.
- if (region == null) {
- region = getS3RegionFromEndpoint(endpointStr,
- endpointEndsWithCentral);
- if (region != null) {
- origin = "endpoint";
- }
- }
+ final RegionResolution.Resolution resolution =
+ calculateRegion(parameters, conf);
+ LOG.debug("Region Resolution: {}", resolution);
- // No need to override endpoint with "s3.amazonaws.com".
- // Let the client take care of endpoint resolution. Overriding
- // the endpoint with "s3.amazonaws.com" causes 400 Bad Request
- // errors for non-existent buckets and objects.
- // ref: https://github.com/aws/aws-sdk-java-v2/issues/4846
- if (!endpointEndsWithCentral) {
- builder.endpointOverride(endpoint);
- LOG.debug("Setting endpoint to {}", endpoint);
- } else {
- origin = "central endpoint with cross region access";
- LOG.debug("Enabling cross region access for endpoint {}",
- endpointStr);
- }
- }
+ // always setting to true or false guarantees the value is non-null,
+ // which tests expect.
+ builder.fipsEnabled(resolution.isUseFips());
- if (region != null) {
- builder.region(region);
- } else if (configuredRegion == null) {
- // no region is configured, and none could be determined from the
endpoint.
- // Use US_EAST_2 as default.
- region = Region.of(AWS_S3_DEFAULT_REGION);
- builder.region(region);
- origin = "cross region access fallback";
- } else if (configuredRegion.isEmpty()) {
+ final RegionResolution.RegionResolutionMechanism mechanism =
resolution.getMechanism();
+ if (Sdk == mechanism) {
+ // handing off all resolution to SDK.
// region configuration was set to empty string.
// allow this if people really want it; it is OK to rely on this
// when deployed in EC2.
- WARN_OF_DEFAULT_REGION_CHAIN.warn(SDK_REGION_CHAIN_IN_USE);
+ DEFAULT_REGION_CHAIN.info(SDK_REGION_CHAIN_IN_USE);
Review Comment:
So we tell sdk to determine the region by not setting anything in the
builder.
> S3A: add explicit "sdk" and "ec2" regions for region resolution through SDK
> and EC2 IAM
> ---------------------------------------------------------------------------------------
>
> Key: HADOOP-19740
> URL: https://issues.apache.org/jira/browse/HADOOP-19740
> Project: Hadoop Common
> Issue Type: Improvement
> Components: fs/s3
> Affects Versions: 3.4.2
> Reporter: Steve Loughran
> Assignee: Steve Loughran
> Priority: Major
> Labels: pull-request-available
>
> Add explicit regions to hand off to the sdk
> * sdk: "use the sdk chain"
> * ec2: "we are in EC2, use the local region": use the iAM logic inside the
> SDK directly.
> empty string "" also hands off to the SDK; the warning will be removed
> also: if an endpoint is set and it is not parsed as a vpce endpoint, we will
> automatically add the endpoint name "external". This avoids the need to make
> up an external region when working with an endpoint.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
