Merge pull request #633 from olamy/enforce-region

Enforce region for aws sdk calls

Author: jglick

src/main/java/io/jenkins/plugins/artifact_manager_jclouds/s3/S3BlobStore.java

@@ -100,7 +100,7 @@ public String getContainer() {
     }
 
     public String getRegion() {
-        return CredentialsAwsGlobalConfiguration.get().getRegion();
+        return getConfiguration().getRegion().id();
     }
 
     public S3BlobStoreConfig getConfiguration(){
@@ -215,6 +215,7 @@ public S3Presigner getS3Presigner(S3Client s3Client) {
         String customEndpoint = getConfiguration().getResolvedCustomEndpoint();
         S3Presigner.Builder presignerBuilder = S3Presigner.builder()
                 .fipsEnabled(FIPS140.useCompliantAlgorithms())
+                .region(getConfiguration().getRegion())
                 .credentialsProvider(CredentialsAwsGlobalConfiguration.get().getCredentials())
                 .s3Client(s3Client);
         if (StringUtils.isNotBlank(customEndpoint)) {
@@ -223,7 +224,7 @@ public S3Presigner getS3Presigner(S3Client s3Client) {
 
         String customRegion = getConfiguration().getCustomSigningRegion();
         if(StringUtils.isBlank(customRegion)) {
-            customRegion = CredentialsAwsGlobalConfiguration.get().getRegion();
+            customRegion = getConfiguration().getRegion().id();
         }
         if(StringUtils.isNotBlank(customRegion)) {
             presignerBuilder.region(Region.of(customRegion));

src/main/java/io/jenkins/plugins/artifact_manager_jclouds/s3/S3BlobStoreConfig.java

@@ -27,6 +27,7 @@
 import java.io.IOException;
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.util.logging.Logger;
 import java.util.regex.Pattern;
 
 import jenkins.util.SystemProperties;
@@ -55,8 +56,10 @@
 import org.jenkinsci.Symbol;
 import software.amazon.awssdk.auth.credentials.AwsSessionCredentials;
 import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider;
+import software.amazon.awssdk.core.exception.SdkClientException;
 import software.amazon.awssdk.core.SdkSystemSetting;
 import software.amazon.awssdk.regions.Region;
+import software.amazon.awssdk.regions.providers.DefaultAwsRegionProviderChain;
 import software.amazon.awssdk.services.s3.S3Client;
 import software.amazon.awssdk.services.s3.S3ClientBuilder;
 import software.amazon.awssdk.services.s3.model.Bucket;
@@ -73,6 +76,8 @@
 @Extension
 public final class S3BlobStoreConfig extends AbstractAwsGlobalConfiguration {
 
+    private static final Logger LOGGER = Logger.getLogger(S3BlobStoreConfig.class.getName());
+
     private static final String BUCKET_REGEXP = "^([a-z]|(\\d(?!\\d{0,2}\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3})))([a-z\\d]|(\\.(?!(\\.|-)))|(-(?!\\.))){1,61}[a-z\\d\\.]$";
     private static final Pattern bucketPattern = Pattern.compile(BUCKET_REGEXP);
 
@@ -291,13 +296,15 @@ S3ClientBuilder getAmazonS3ClientBuilder() throws URISyntaxException {
         if (StringUtils.isNotBlank(getResolvedCustomEndpoint())) {
             String resolvedCustomSigningRegion = customSigningRegion;
             if (StringUtils.isBlank(resolvedCustomSigningRegion)) {
-                resolvedCustomSigningRegion = "us-east-1";
+                // we must revert to a region if no custom defined
+                resolvedCustomSigningRegion = getRegion().id();
             }
             ret = ret.endpointOverride(new URI(getResolvedCustomEndpoint())).region(Region.of(resolvedCustomSigningRegion));
-        } else if (StringUtils.isNotBlank(CredentialsAwsGlobalConfiguration.get().getRegion())) {
-            ret = ret.region(Region.of(CredentialsAwsGlobalConfiguration.get().getRegion()));
         } else {
-            ret = ret.useArnRegion(true);
+            // not really sure of why this was used.. this should have a dedicated parameter
+            //ret = ret.useArnRegion(true);
+            // use same default algorithm as signer
+            ret = ret.region(getRegion());
         }
         ret = ret.accelerate(useTransferAcceleration);
 
@@ -316,20 +323,34 @@ public S3ClientBuilder getAmazonS3ClientBuilderWithCredentials() throws IOExcept
         }
     }
 
+    public Region getRegion() {
+        String regionStr = CredentialsAwsGlobalConfiguration.get().getRegion();
+        if (regionStr == null) {
+            try {
+                return new DefaultAwsRegionProviderChain().getRegion();
+            } catch (SdkClientException e) {
+                // need to revert to a default one.
+                LOGGER.warning("There is no region configured for this S3 bucket and aws sdk couldn't discover one so using default: " + Region.US_EAST_1);
+                return Region.US_EAST_1;
+            }
+        }
+        return Region.of(regionStr);
+    }
+
     private S3ClientBuilder getAmazonS3ClientBuilderWithCredentials(boolean disableSessionToken) throws IOException, URISyntaxException {
         S3ClientBuilder builder = getAmazonS3ClientBuilder();
         if (disableSessionToken) {
             builder = builder.credentialsProvider(CredentialsAwsGlobalConfiguration.get().getCredentials());
         } else {
             AwsSessionCredentials awsSessionCredentials = CredentialsAwsGlobalConfiguration.get()
-                    .sessionCredentials(CredentialsAwsGlobalConfiguration.get().getRegion(),
-                            CredentialsAwsGlobalConfiguration.get().getCredentialsId());
+                    .sessionCredentials(getRegion().id(), CredentialsAwsGlobalConfiguration.get().getCredentialsId());
             if(awsSessionCredentials != null ) {
                 builder.credentialsProvider(StaticCredentialsProvider.create(awsSessionCredentials));
             } else {
                 throw new IOException("No session AWS credentials found");
             }
         }
+        builder = builder.region(getRegion());
         return builder;
     }
 

src/test/java/io/jenkins/plugins/artifact_manager_jclouds/s3/JCloudsVirtualFileTest.java

@@ -28,7 +28,6 @@
 import io.jenkins.plugins.aws.global_configuration.CredentialsAwsGlobalConfiguration;
 import static org.hamcrest.Matchers.*;
 import static org.jclouds.blobstore.options.ListContainerOptions.Builder.*;
-import static org.junit.Assert.*;
 
 import java.io.File;
 import java.io.FileNotFoundException;
@@ -56,7 +55,14 @@
 import java.net.ProtocolException;
 
 import jenkins.util.VirtualFile;
+import static org.hamcrest.MatcherAssert.assertThat;
 import org.jclouds.http.HttpResponseException;
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 import software.amazon.awssdk.services.s3.S3Client;
 import software.amazon.awssdk.services.s3.model.ListObjectsV2Request;
 import software.amazon.awssdk.services.s3.model.ListObjectsV2Response;
@@ -248,7 +254,7 @@ public void pagedListing() throws Exception {
         });
         // Default list page size for S3 is 1000 blobs; we have 1010 plus the two created for all tests, so should hit a second page.
         assertThat(subdir.list("sprawling/**/k3", null, true), iterableWithSize(100));
-        assertEquals("calls GetBucketLocation then ListBucket, advance to …/sprawling/i9/j8/k8, ListBucket again", 3, httpLogging.getRecords().size());
+        assertThat("calls GetBucketLocation (perhaps) then ListBucket, advance to …/sprawling/i9/j8/k8, ListBucket again", httpLogging.getRecords().size(), lessThanOrEqualTo(3));
     }
 
     @Test