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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
