Skip to content

Commit 0989d31

Browse files
jorgeepditommaso
andauthored
Fix incorrect AWS region when specifying a S3 regional endpoint (#6530)
Signed-off-by: jorgee <[email protected]> Signed-off-by: Jorge Ejarque <[email protected]> Co-authored-by: Paolo Di Tommaso <[email protected]>
1 parent f4548bd commit 0989d31

File tree

5 files changed

+78
-1
lines changed

5 files changed

+78
-1
lines changed

plugins/nf-amazon/src/main/nextflow/cloud/aws/config/AwsConfig.groovy

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,12 +93,29 @@ class AwsConfig implements ConfigScope {
9393

9494
AwsBatchConfig getBatchConfig() { batch }
9595

96+
@Deprecated
9697
String getS3GlobalRegion() {
9798
return !region || !s3Config.endpoint || s3Config.endpoint.contains(".amazonaws.com")
9899
? Region.US_EAST_1.id() // always use US_EAST_1 as global region for AWS endpoints
99100
: region // for custom endpoint use the config provided region
100101
}
101102

103+
/**
104+
* Resolves the region used for S3 evaluating the region resolved from config and a possible region defined in the endpoint.
105+
* Fallback to the global region US_EAST_1 when no region is found.
106+
*
107+
* Preference:
108+
* 1. endpoint region
109+
* 2. config region
110+
* 3. US_EAST_1
111+
*
112+
* @returns Resolved region.
113+
**/
114+
String resolveS3Region() {
115+
final epRegion = client.getEndpointRegion()
116+
return epRegion ?: this.region ?: Region.US_EAST_1.id()
117+
}
118+
102119
static protected String getAwsProfile0(Map env, Map<String,Object> config) {
103120

104121
final profile = config?.profile as String

plugins/nf-amazon/src/main/nextflow/cloud/aws/config/AwsS3Config.groovy

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package nextflow.cloud.aws.config
1919

2020
import static nextflow.cloud.aws.util.AwsHelper.*
2121

22+
import software.amazon.awssdk.regions.Region
2223
import software.amazon.awssdk.services.s3.model.ObjectCannedACL
2324
import groovy.transform.CompileStatic
2425
import groovy.util.logging.Slf4j
@@ -288,6 +289,35 @@ class AwsS3Config implements ConfigScope {
288289
endpoint && !endpoint.endsWith(".amazonaws.com")
289290
}
290291

292+
/**
293+
* Looks for the region defined in endpoints such as https://xxx.<region>.amazonaws.com
294+
* @returns Region defined in the endpoint. Null if no endpoint or custom endpoint is defined,
295+
* or when URI region subdomain doesn't match with a region (global or multi-region access point)
296+
*/
297+
String getEndpointRegion(){
298+
if( !endpoint || isCustomEndpoint() )
299+
return null
300+
301+
try {
302+
String host = URI.create(endpoint).getHost()
303+
final hostDomains = host.split('\\.')
304+
if (hostDomains.size() < 3) {
305+
log.debug("Region subdomain doesn't exist in endpoint '${endpoint}'")
306+
return null
307+
}
308+
final region = hostDomains[hostDomains.size()-3]
309+
if (!Region.regions().contains(Region.of(region))){
310+
log.debug("Region '${region}' extracted from endpoint '${endpoint}' is not valid")
311+
return null
312+
}
313+
return region
314+
315+
} catch (Exception e){
316+
log.debug("Exception getting region from endpoint: '${endpoint}' - ${e.message}")
317+
return null
318+
}
319+
}
320+
291321
Map<String,String> getAwsClientConfig() {
292322
return [
293323
connection_timeout: connectionTimeout?.toString(),

plugins/nf-amazon/src/main/nextflow/cloud/aws/nio/S3FileSystemProvider.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -740,7 +740,7 @@ protected S3FileSystem createFileSystem(URI uri, AwsConfig awsConfig) {
740740
// when enabling that flag, it overrides S3 endpoints with AWS global endpoint
741741
// see https://github.com/nextflow-io/nextflow/pull/5779
742742
final boolean global = bucketName!=null && !awsConfig.getS3Config().isCustomEndpoint();
743-
final AwsClientFactory factory = new AwsClientFactory(awsConfig, awsConfig.getS3GlobalRegion());
743+
final AwsClientFactory factory = new AwsClientFactory(awsConfig, awsConfig.resolveS3Region());
744744
final S3Client client = new S3Client(factory, props, global);
745745

746746
// set the client acl

plugins/nf-amazon/src/test/nextflow/cloud/aws/config/AwsConfigTest.groovy

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717

1818
package nextflow.cloud.aws.config
1919

20+
import software.amazon.awssdk.regions.Region
21+
2022
import java.nio.file.Files
2123

2224
import nextflow.SysEnv
@@ -115,4 +117,18 @@ class AwsConfigTest extends Specification {
115117
[max_error_retry: '2', foo: 1] | [:] | [max_error_retry: '2', foo: 1]
116118
[:] | [:] | [max_error_retry: '5']
117119
}
120+
121+
@Unroll
122+
def 'should resolve S3 region' () {
123+
expect:
124+
new AwsConfig(CONFIG).resolveS3Region() == REGION
125+
126+
where:
127+
CONFIG | REGION
128+
[:] | Region.US_EAST_1.id()
129+
[client: [endpoint: "http://custom.endpoint.com"]] | Region.US_EAST_1.id()
130+
[region: "eu-south-1", client: [endpoint: "http://custom.endpoint.com"]] | Region.EU_SOUTH_1.id()
131+
[region: "eu-south-1", client: [endpoint: "https://s3.eu-west-1.amazonaws.com"]] | Region.EU_WEST_1.id()
132+
[region: "eu-south-1", client: [endpoint: "https://bucket.s3-global.amazonaws.com"]] | Region.EU_SOUTH_1.id()
133+
}
118134
}

plugins/nf-amazon/src/test/nextflow/cloud/aws/config/AwsS3ConfigTest.groovy

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
package nextflow.cloud.aws.config
1919

20+
import software.amazon.awssdk.regions.Region
2021
import software.amazon.awssdk.services.s3.model.ObjectCannedACL
2122
import nextflow.SysEnv
2223
import spock.lang.Specification
@@ -189,4 +190,17 @@ class AwsS3ConfigTest extends Specification {
189190
[ maxDownloadHeapMemory: '50 MB', minimumPartSize: '6 MB'] | "Configuration option `aws.client.maxDownloadHeapMemory` must be at least 10 times `aws.client.minimumPartSize`"
190191
}
191192

193+
@Unroll
194+
def 'should get region from endpoint' () {
195+
expect:
196+
new AwsS3Config(CONFIG).getEndpointRegion() == REGION
197+
198+
where:
199+
CONFIG | REGION
200+
[:] | null
201+
[endpoint: "http://custom.endpoint.com"] | null
202+
[endpoint: "https://s3.eu-west-1.amazonaws.com"] | Region.EU_WEST_1.id()
203+
[endpoint: "https://bucket.s3-global.amazonaws.com"] | null
204+
}
205+
192206
}

0 commit comments

Comments
 (0)