Fix #1402 Migrating AWS S3 SDK to v2 (#1403)

slackapi · Dec 6, 2024 · 26dba31 · 26dba31
1 parent 5b0212f
commit 26dba31
Show file tree

Hide file tree

Showing 11 changed files with 236 additions and 207 deletions.
diff --git a/bolt-jakarta-servlet/pom.xml b/bolt-jakarta-servlet/pom.xml
@@ -61,8 +61,8 @@
             <scope>test</scope>
         </dependency>
         <dependency>
-            <groupId>com.amazonaws</groupId>
-            <artifactId>aws-java-sdk-s3</artifactId>
+            <groupId>software.amazon.awssdk</groupId>
+            <artifactId>s3</artifactId>
             <version>${aws.s3.version}</version>
             <scope>test</scope>
         </dependency>

diff --git a/bolt-kotlin-examples/pom.xml b/bolt-kotlin-examples/pom.xml
@@ -41,8 +41,8 @@
         </dependency>
 
         <dependency>
-            <groupId>com.amazonaws</groupId>
-            <artifactId>aws-java-sdk-s3</artifactId>
+            <groupId>software.amazon.awssdk</groupId>
+            <artifactId>s3</artifactId>
             <version>${aws.s3.version}</version>
         </dependency>
 

diff --git a/bolt-servlet/pom.xml b/bolt-servlet/pom.xml
@@ -66,8 +66,8 @@
             <scope>test</scope>
         </dependency>
         <dependency>
-            <groupId>com.amazonaws</groupId>
-            <artifactId>aws-java-sdk-s3</artifactId>
+            <groupId>software.amazon.awssdk</groupId>
+            <artifactId>s3</artifactId>
             <version>${aws.s3.version}</version>
             <scope>test</scope>
         </dependency>

diff --git a/bolt/pom.xml b/bolt/pom.xml
@@ -41,8 +41,8 @@
         </dependency>
 
         <dependency>
-            <groupId>com.amazonaws</groupId>
-            <artifactId>aws-java-sdk-s3</artifactId>
+            <groupId>software.amazon.awssdk</groupId>
+            <artifactId>s3</artifactId>
             <version>${aws.s3.version}</version>
             <scope>provided</scope>
         </dependency>

diff --git a/bolt/src/main/java/com/slack/api/bolt/service/builtin/AmazonS3InstallationService.java b/bolt/src/main/java/com/slack/api/bolt/service/builtin/AmazonS3InstallationService.java
diff --git a/bolt/src/main/java/com/slack/api/bolt/service/builtin/AmazonS3OAuthStateService.java b/bolt/src/main/java/com/slack/api/bolt/service/builtin/AmazonS3OAuthStateService.java
@@ -1,19 +1,18 @@
 package com.slack.api.bolt.service.builtin;
 
-import com.amazonaws.auth.AWSCredentials;
-import com.amazonaws.auth.DefaultAWSCredentialsProviderChain;
-import com.amazonaws.services.s3.AmazonS3;
-import com.amazonaws.services.s3.AmazonS3ClientBuilder;
-import com.amazonaws.services.s3.model.AmazonS3Exception;
-import com.amazonaws.services.s3.model.PutObjectResult;
-import com.amazonaws.services.s3.model.S3Object;
-import com.amazonaws.util.IOUtils;
 import com.slack.api.bolt.Initializer;
 import com.slack.api.bolt.service.OAuthStateService;
-import com.slack.api.bolt.util.JsonOps;
 import lombok.extern.slf4j.Slf4j;
+import software.amazon.awssdk.auth.credentials.AwsCredentials;
+import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider;
+import software.amazon.awssdk.auth.credentials.DefaultCredentialsProvider;
+import software.amazon.awssdk.core.ResponseBytes;
+import software.amazon.awssdk.core.sync.RequestBody;
+import software.amazon.awssdk.core.sync.ResponseTransformer;
+import software.amazon.awssdk.services.s3.S3Client;
+import software.amazon.awssdk.services.s3.model.*;
 
-import java.io.IOException;
+import java.nio.charset.StandardCharsets;
 
 /**
  * OAuthStateService implementation using Amazon S3.
@@ -24,6 +23,7 @@
 public class AmazonS3OAuthStateService implements OAuthStateService {
 
     private final String bucketName;
+    private AwsCredentialsProvider credentialsProvider;
 
     public AmazonS3OAuthStateService(String bucketName) {
         this.bucketName = bucketName;
@@ -33,74 +33,92 @@ public AmazonS3OAuthStateService(String bucketName) {
     public Initializer initializer() {
         return (app) -> {
             // The first access to S3 tends to be slow on AWS Lambda.
-            AWSCredentials credentials = getCredentials();
-            if (credentials == null || credentials.getAWSAccessKeyId() == null) {
+            this.credentialsProvider = DefaultCredentialsProvider.create();
+            AwsCredentials credentials = createCredentials(credentialsProvider);
+            if (credentials == null || credentials.accessKeyId() == null) {
                 throw new IllegalStateException("AWS credentials not found");
             }
             if (log.isDebugEnabled()) {
-                log.debug("AWS credentials loaded (access key id: {})", credentials.getAWSAccessKeyId());
+                log.debug("AWS credentials loaded (access key id: {})", credentials.accessKeyId());
+            }
+            boolean bucketExists = false;
+            Exception ex = null;
+            try (S3Client s3 = createS3Client()) {
+                bucketExists = s3.headBucket(HeadBucketRequest.builder().bucket(bucketName).build()) != null;
+            } catch (Exception e) { // NoSuchBucketException etc.
+                ex = e;
             }
-            boolean bucketExists = createS3Client().doesBucketExistV2(bucketName);
             if (!bucketExists) {
-                throw new IllegalStateException("Failed to access the Amazon S3 bucket (name: " + bucketName + ")");
+                String error = ex != null ? ex.getClass().getName() + ":" + ex.getMessage() : "-";
+                String message = "Failed to access the Amazon S3 bucket (name: " + bucketName + ", error: " + error + ")";
+                throw new IllegalStateException(message);
             }
         };
     }
 
     @Override
     public void addNewStateToDatastore(String state) throws Exception {
-        AmazonS3 s3 = this.createS3Client();
-        String value = "" + (System.currentTimeMillis() + getExpirationInSeconds() * 1000);
-        PutObjectResult putObjectResult = s3.putObject(bucketName, getKey(state), value);
+        PutObjectResponse putObjectResult;
+        try (S3Client s3 = this.createS3Client()) {
+            String value = "" + (System.currentTimeMillis() + getExpirationInSeconds() * 1000);
+            putObjectResult = s3.putObject(
+                    PutObjectRequest.builder().bucket(bucketName).key(getKey(state)).build(),
+                    RequestBody.fromString(value)
+            );
+        }
         if (log.isDebugEnabled()) {
-            log.debug("AWS S3 putObject result of state data - {}", JsonOps.toJsonString(putObjectResult));
+            log.debug("AWS S3 putObject result of state data - {}", putObjectResult.toString());
         }
     }
 
     @Override
     public boolean isAvailableInDatabase(String state) {
-        AmazonS3 s3 = this.createS3Client();
-        S3Object s3Object = getObject(s3, getKey(state));
+        S3Client s3 = this.createS3Client();
+        ResponseBytes<GetObjectResponse> s3Object = getObject(s3, getKey(state));
         if (s3Object == null) {
             return false;
         }
         String millisToExpire = null;
         try {
-            millisToExpire = IOUtils.toString(s3Object.getObjectContent());
-            return Long.valueOf(millisToExpire) > System.currentTimeMillis();
-        } catch (IOException e) {
-            log.error("Failed to load a state data for state: {}", state, e);
-            return false;
+            millisToExpire = s3Object.asString(StandardCharsets.UTF_8);
+            return Long.parseLong(millisToExpire) > System.currentTimeMillis();
         } catch (NumberFormatException ne) {
             log.error("Invalid state value detected - state: {}, millisToExpire: {}", state, millisToExpire);
             return false;
+        } catch (Exception e) {
+            log.error("Failed to load a state data for state: {}", state, e);
+            return false;
         }
     }
 
     @Override
     public void deleteStateFromDatastore(String state) throws Exception {
-        AmazonS3 s3 = this.createS3Client();
-        s3.deleteObject(bucketName, getKey(state));
+        try (S3Client s3 = this.createS3Client()) {
+            s3.deleteObject(DeleteObjectRequest.builder().bucket(bucketName).key(getKey(state)).build());
+        }
     }
 
-    protected AWSCredentials getCredentials() {
-        return DefaultAWSCredentialsProviderChain.getInstance().getCredentials();
+    protected AwsCredentials createCredentials(AwsCredentialsProvider provider) {
+        return provider.resolveCredentials();
     }
 
-    protected AmazonS3 createS3Client() {
-        return AmazonS3ClientBuilder.defaultClient();
+    protected S3Client createS3Client() {
+        return S3Client.builder().credentialsProvider(this.credentialsProvider).build();
     }
 
     private String getKey(String state) {
         return "state/" + state;
     }
 
-    private S3Object getObject(AmazonS3 s3, String fullKey) {
+    private ResponseBytes<GetObjectResponse> getObject(S3Client s3, String fullKey) {
         try {
-            return s3.getObject(bucketName, fullKey);
-        } catch (AmazonS3Exception e) {
+            return s3.getObject(
+                    GetObjectRequest.builder().bucket(bucketName).key(fullKey).build(),
+                    ResponseTransformer.toBytes()
+            );
+        } catch (Exception e) {
             if (log.isDebugEnabled()) {
-                log.debug("Amazon S3 object metadata not found (key: {}, AmazonS3Exception: {})", fullKey, e.toString());
+                log.debug("Amazon S3 object metadata not found (key: {}, Exception: {})", fullKey, e.toString());
             }
             return null;
         }

diff --git a/bolt/src/test/java/test_locally/service/AmazonS3InstallationServiceTest.java b/bolt/src/test/java/test_locally/service/AmazonS3InstallationServiceTest.java
@@ -1,47 +1,44 @@
 package test_locally.service;
 
-import com.amazonaws.SdkClientException;
-import com.amazonaws.auth.AWSCredentials;
-import com.amazonaws.services.s3.AmazonS3;
 import com.slack.api.bolt.model.builtin.DefaultBot;
 import com.slack.api.bolt.model.builtin.DefaultInstaller;
 import com.slack.api.bolt.service.builtin.AmazonS3InstallationService;
 import org.junit.Test;
+import software.amazon.awssdk.auth.credentials.AwsCredentials;
+import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider;
+import software.amazon.awssdk.core.ResponseBytes;
+import software.amazon.awssdk.core.sync.RequestBody;
+import software.amazon.awssdk.core.sync.ResponseTransformer;
+import software.amazon.awssdk.services.s3.S3Client;
+import software.amazon.awssdk.services.s3.model.*;
+
+import java.nio.charset.StandardCharsets;
 
 import static org.junit.Assert.assertNotNull;
 import static org.mockito.Mockito.*;
 
 public class AmazonS3InstallationServiceTest {
 
-    @Test(expected = IllegalStateException.class)
-    public void initializer_no_credentials() {
-        AmazonS3 s3 = mock(AmazonS3.class);
-        AmazonS3InstallationService service = new AmazonS3InstallationService("test-bucket") {
-            @Override
-            protected AWSCredentials getCredentials() {
-                return null;
-            }
-
-            @Override
-            protected AmazonS3 createS3Client() {
-                return s3;
-            }
-        };
-        service.initializer().accept(null);
+    static S3Client setupMocks(S3Client s3) {
+        when(s3.headBucket((HeadBucketRequest) notNull())).thenReturn(HeadBucketResponse.builder().build());
+        when(s3.headObject((HeadObjectRequest) notNull())).thenReturn(HeadObjectResponse.builder().build());
+        when(s3.getObject((GetObjectRequest) notNull(), (ResponseTransformer<GetObjectResponse, ResponseBytes<GetObjectResponse>>) notNull()))
+                .thenReturn(ResponseBytes.fromByteArray(GetObjectResponse.builder().build(), "{}".getBytes(StandardCharsets.UTF_8)));
+        when(s3.putObject((PutObjectRequest) notNull(), (RequestBody) notNull())).thenReturn(PutObjectResponse.builder().build());
+        return s3;
     }
 
     @Test(expected = IllegalStateException.class)
-    public void initializer_no_bucket() {
-        AWSCredentials credentials = mock(AWSCredentials.class);
-        AmazonS3 s3 = mock(AmazonS3.class);
+    public void initializer_no_credentials() {
+        S3Client s3 = setupMocks(mock(S3Client.class));
         AmazonS3InstallationService service = new AmazonS3InstallationService("test-bucket") {
             @Override
-            protected AWSCredentials getCredentials() {
-                return credentials;
+            protected AwsCredentials createCredentials(AwsCredentialsProvider provider) {
+                return null;
             }
 
             @Override
-            protected AmazonS3 createS3Client() {
+            protected S3Client createS3Client() {
                 return s3;
             }
         };
@@ -50,19 +47,18 @@ protected AmazonS3 createS3Client() {
 
     @Test
     public void initializer() {
-        AWSCredentials credentials = mock(AWSCredentials.class);
-        when(credentials.getAWSAccessKeyId()).thenReturn("valid key");
-        AmazonS3 s3 = mock(AmazonS3.class);
-        when(s3.doesBucketExistV2(anyString())).thenReturn(true);
+        AwsCredentials credentials = mock(AwsCredentials.class);
+        when(credentials.accessKeyId()).thenReturn("valid key");
+        S3Client s3 = setupMocks(mock(S3Client.class));
 
         AmazonS3InstallationService service = new AmazonS3InstallationService("test-bucket") {
             @Override
-            protected AWSCredentials getCredentials() {
+            protected AwsCredentials createCredentials(AwsCredentialsProvider provider) {
                 return credentials;
             }
 
             @Override
-            protected AmazonS3 createS3Client() {
+            protected S3Client createS3Client() {
                 return s3;
             }
         };
@@ -71,19 +67,18 @@ protected AmazonS3 createS3Client() {
 
     @Test
     public void operations() throws Exception {
-        AWSCredentials credentials = mock(AWSCredentials.class);
-        when(credentials.getAWSAccessKeyId()).thenReturn("valid key");
-        AmazonS3 s3 = mock(AmazonS3.class);
-        when(s3.doesBucketExistV2(anyString())).thenReturn(true);
+        AwsCredentials credentials = mock(AwsCredentials.class);
+        when(credentials.accessKeyId()).thenReturn("valid key");
+        S3Client s3 = setupMocks(mock(S3Client.class));
 
         AmazonS3InstallationService service = new AmazonS3InstallationService("test-bucket") {
             @Override
-            protected AWSCredentials getCredentials() {
+            protected AwsCredentials createCredentials(AwsCredentialsProvider provider) {
                 return credentials;
             }
 
             @Override
-            protected AmazonS3 createS3Client() {
+            protected S3Client createS3Client() {
                 return s3;
             }
         };
@@ -98,19 +93,18 @@ protected AmazonS3 createS3Client() {
 
     @Test
     public void operations_historical_data_enabled() throws Exception {
-        AWSCredentials credentials = mock(AWSCredentials.class);
-        when(credentials.getAWSAccessKeyId()).thenReturn("valid key");
-        AmazonS3 s3 = mock(AmazonS3.class);
-        when(s3.doesBucketExistV2(anyString())).thenReturn(true);
+        AwsCredentials credentials = mock(AwsCredentials.class);
+        when(credentials.accessKeyId()).thenReturn("valid key");
+        S3Client s3 = setupMocks(mock(S3Client.class));
 
         AmazonS3InstallationService service = new AmazonS3InstallationService("test-bucket") {
             @Override
-            protected AWSCredentials getCredentials() {
+            protected AwsCredentials createCredentials(AwsCredentialsProvider provider) {
                 return credentials;
             }
 
             @Override
-            protected AmazonS3 createS3Client() {
+            protected S3Client createS3Client() {
                 return s3;
             }
         };
@@ -129,32 +123,18 @@ public MyService(String bucketName) {
             super(bucketName);
         }
 
-        public AWSCredentials credentials() {
-            return getCredentials();
-        }
-
-        public AmazonS3 s3() {
+        public S3Client s3() {
             return createS3Client();
         }
     }
 
-    @Test
-    public void credentials() {
-        MyService service = new MyService("test-bucket");
-        try {
-            AWSCredentials credentials = service.credentials();
-            assertNotNull(credentials);
-        } catch (SdkClientException ignored) {
-        }
-    }
-
     @Test
     public void s3() {
         MyService service = new MyService("test-bucket");
         try {
-            AmazonS3 s3 = service.s3();
+            S3Client s3 = service.s3();
             assertNotNull(s3);
-        } catch (SdkClientException ignored) {
+        } catch (Exception ignored) {
         }
     }