Skip to content

Commit a19249c

Browse files
max-zillamax-zilla
authored
Merge branch 'develop' into 277-Dataset-Thumbnail-404-Not-Found---Broken-Link-after-File-Deletion
2 parents ceb07da + 276434f commit a19249c

File tree

5 files changed

+165
-106
lines changed

5 files changed

+165
-106
lines changed

‎CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,18 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
66

77
## Unreleased
88

9+
### Added
10+
- Support the [DefaultAWSCredentialsProviderChain](https://docs.aws.amazon.com/AWSJavaSDK/latest/javadoc/com/amazonaws/auth/DefaultAWSCredentialsProviderChain.html)
11+
for passing in credentials to the S3ByteStorageService.
12+
913
### Fixed
1014
- Upgraded extractor parameters jsonform to version `2.2.5`.
15+
- Cleaning up after a failed upload should no longer decrement the file + byte counts.
1116
- Fix the broken link after file deletion within a folder. [#277](https://github.com/clowder-framework/clowder/issues/277)
1217

1318
### Changed
1419
- now building mongo-init and monitor docker containers with python 3.8
20+
- Upgraded extractor parameters jsonform to version `2.2.5`.
1521

1622
### Removed
1723
- check image is now part of [ncsa/checks](https://github.com/ncsa/checks/)

‎app/services/mongodb/MongoDBFileService.scala‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -847,20 +847,24 @@ class MongoDBFileService @Inject() (
847847
}
848848
}
849849

850+
850851
// delete the actual file
851-
if(isLastPointingToLoader(file.loader, file.loader_id)) {
852+
val fileSize = if(isLastPointingToLoader(file.loader, file.loader_id)) {
852853
for(preview <- previews.findByFileId(file.id)){
853854
previews.removePreview(preview)
854855
}
855856
if(!file.thumbnail_id.isEmpty)
856857
thumbnails.remove(UUID(file.thumbnail_id.get))
857858
ByteStorageService.delete(file.loader, file.loader_id, FileDAO.COLLECTION)
859+
file.length
860+
} else {
861+
0
858862
}
859863

860864
import UUIDConversions._
861865
FileDAO.removeById(file.id)
862866
appConfig.incrementCount('files, -1)
863-
appConfig.incrementCount('bytes, -file.length)
867+
appConfig.incrementCount('bytes, -1 * fileSize)
864868
current.plugin[ElasticsearchPlugin].foreach {
865869
_.delete(id.stringify)
866870
}

‎app/services/s3/S3ByteStorageService.scala‎

Lines changed: 124 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,41 @@
11
package services.s3
22

3-
import java.io.{File, FileOutputStream, IOException, InputStream}
3+
import java.io.{IOException, InputStream}
44
import models.UUID
5-
6-
import com.amazonaws.auth.{AWSStaticCredentialsProvider, BasicAWSCredentials}
5+
import com.amazonaws.auth.{AWSCredentialsProviderChain, AWSStaticCredentialsProvider, BasicAWSCredentials, DefaultAWSCredentialsProviderChain}
76
import com.amazonaws.client.builder.AwsClientBuilder
87
import com.amazonaws.regions.Regions
9-
import com.amazonaws.services.s3.model.{HeadBucketRequest, CreateBucketRequest, GetObjectRequest, ObjectMetadata}
8+
import com.amazonaws.services.s3.model.{CreateBucketRequest, GetObjectRequest, HeadBucketRequest, ObjectMetadata}
109
import com.amazonaws.services.s3.{AmazonS3, AmazonS3ClientBuilder}
1110
import com.amazonaws.{AmazonClientException, ClientConfiguration}
1211
import com.google.inject.Inject
1312
import play.Logger
1413
import play.api.Play
1514
import services.ByteStorageService
1615
import com.amazonaws.AmazonServiceException
17-
import com.amazonaws.services.s3.transfer.TransferManager
1816
import com.amazonaws.services.s3.transfer.TransferManagerBuilder
1917
import com.amazonaws.services.s3.transfer.Upload
18+
import services.s3.S3ByteStorageService.{handleACE, handleASE, handleIOE, handleUnknownError}
19+
20+
21+
/**
22+
* A ByteStorageService for Clowder that enables use of S3-compatible
23+
* object stores to serve as the file backing for Clowder. This allows
24+
* you to use an S3 bucket on AWS or Minio to store your files.
25+
*
26+
*
27+
* Available Configuration Options:
28+
* clowder.s3.serviceEndpoint - Host/port of the service to use for storage
29+
* clowder.s3.bucketName - the name of the bucket that should be used to store files
30+
* clowder.s3.accessKey - access key with which to access the bucket
31+
* clowder.s3.secretKey - secret key associated with the access key above
32+
* clowder.s3.region - the region where your S3 bucket lives
33+
* clowder.s3.depth - the number of sub-paths to insert (default: 3)
34+
* NOTE: this will randomly distribute files into smaller subdirectories and is recommended for performance reasons
35+
*
36+
* @author Mike Lambert
37+
*
38+
*/
2039

2140
/** Available configuration options for s3 storage */
2241
object S3ByteStorageService {
@@ -25,42 +44,120 @@ object S3ByteStorageService {
2544
val AccessKey: String = "clowder.s3.accessKey"
2645
val SecretKey: String = "clowder.s3.secretKey"
2746
val Region: String = "clowder.s3.region"
47+
48+
/* Reusable handlers for various Exception types */
49+
def handleUnknownError(err: Exception = null) = {
50+
if (err != null) {
51+
Logger.error("An unknown error occurred in the S3ByteStorageService: " + err.toString)
52+
} else {
53+
Logger.error("An unknown error occurred in the S3ByteStorageService.")
54+
}
55+
}
56+
57+
/* Reusable handlers for various Exception types */
58+
def handleIOE(err: IOException) = {
59+
Logger.error("IOException occurred in the S3ByteStorageService: " + err)
60+
}
61+
62+
/* Reusable handlers for various Exception types */
63+
def handleACE(ace: AmazonClientException) = {
64+
Logger.error("Caught an AmazonClientException, which " + "means the client encountered " + "an internal error while trying to " + "communicate with S3, " + "such as not being able to access the network.")
65+
Logger.error("Error Message: " + ace.getMessage)
66+
}
67+
68+
/* Reusable handlers for various Exception types */
69+
def handleASE(ase: AmazonServiceException) = {
70+
Logger.error("Caught an AmazonServiceException, which " + "means your request made it " + "to Amazon S3, but was rejected with an error response" + " for some reason.")
71+
Logger.error("Error Message: " + ase.getMessage)
72+
Logger.error("HTTP Status Code: " + ase.getStatusCode)
73+
Logger.error("AWS Error Code: " + ase.getErrorCode)
74+
Logger.error("Error Type: " + ase.getErrorType)
75+
Logger.error("Request ID: " + ase.getRequestId)
76+
}
2877
}
2978

3079
/**
80+
*
3181
* A ByteStorageService for Clowder that enables use of S3-compatible
3282
* object stores to serve as the file backing for Clowder. This allows
3383
* you to use an S3 bucket on AWS or Minio to store your files.
34-
*
35-
*
36-
* Available Configuration Options:
37-
* clowder.s3.serviceEndpoint - Host/port of the service to use for storage
38-
* clowder.s3.bucketName - the name of the bucket that should be used to store files
39-
* clowder.s3.accessKey - access key with which to access the bucket
40-
* clowder.s3.secretKey - secret key associated with the access key above
41-
* clowder.s3.region - the region where your S3 bucket lives (currently unused)
42-
*
43-
*
44-
* @author Mike Lambert
45-
*
4684
*/
4785
class S3ByteStorageService @Inject()() extends ByteStorageService {
86+
val s3: AmazonS3 = this.buildS3Client()
87+
this.ensureBucketAccessAndExists()
88+
89+
def buildS3Client(): AmazonS3 = {
90+
// NOTE: Region is ignored for MinIO case
91+
val s3client = (Play.current.configuration.getString(S3ByteStorageService.ServiceEndpoint), Play.current.configuration.getString(S3ByteStorageService.Region)) match {
92+
case (Some(serviceEndpoint), Some(region)) => {
93+
Logger.debug("Creating S3 Client with custom endpoint and region: " + serviceEndpoint + " in region " + region)
94+
AmazonS3ClientBuilder.standard().withEndpointConfiguration(new AwsClientBuilder.EndpointConfiguration(serviceEndpoint, region))
95+
}
96+
case (Some(serviceEndpoint), None) => {
97+
Logger.debug("Creating S3 Client with custom endpoint: " + serviceEndpoint + " (using default region)")
98+
AmazonS3ClientBuilder.standard()
99+
.withEndpointConfiguration(new AwsClientBuilder.EndpointConfiguration(serviceEndpoint, Regions.US_EAST_1.name()))
100+
}
101+
case (None, Some(region)) => {
102+
Logger.debug("Creating S3 Client with custom region: " + region)
103+
AmazonS3ClientBuilder.standard().withRegion(region)
104+
}
105+
case (None, None) => {
106+
Logger.debug("Creating S3 Client with default region.")
107+
AmazonS3ClientBuilder.standard()
108+
}
109+
}
110+
111+
// Search for AccessKey / SecretKey in config (envvar values will be populated here by default)
112+
(Play.current.configuration.getString(S3ByteStorageService.AccessKey),
113+
Play.current.configuration.getString(S3ByteStorageService.SecretKey)) match {
114+
case (Some(accessKey), Some(secretKey)) => {
115+
val credentials = new BasicAWSCredentials(accessKey, secretKey)
116+
val clientConfiguration = new ClientConfiguration
117+
clientConfiguration.setSignerOverride("AWSS3V4SignerType")
48118

119+
Logger.debug("Creating S3 Client with custom credentials.")
49120

50-
// Only run a single thread at a time when verifying bucket existence
51-
synchronized {
121+
return s3client.withClientConfiguration(clientConfiguration)
122+
.withPathStyleAccessEnabled(true)
123+
.withCredentials(new AWSStaticCredentialsProvider(credentials))
124+
.build()
125+
}
126+
case (None, None) => {
127+
Logger.debug("Creating S3 Client with default credentials.")
128+
129+
return s3client.withCredentials(DefaultAWSCredentialsProviderChain.getInstance)
130+
.withPathStyleAccessEnabled(true)
131+
.build()
132+
}
133+
134+
case _ => {
135+
val errMsg = "Bad S3 configuration: accessKey and secretKey are both required if one is given. Falling back to default credentials..."
136+
Logger.warn(errMsg)
137+
138+
return s3client.withCredentials(DefaultAWSCredentialsProviderChain.getInstance)
139+
.withPathStyleAccessEnabled(true)
140+
.build()
141+
}
142+
}
143+
}
144+
145+
def ensureBucketAccessAndExists() = {
146+
// Ensure that bucket exists and that we have access to it before continuing
52147
Play.current.configuration.getString(S3ByteStorageService.BucketName) match {
53148
case Some(bucketName) => {
54149
try {
55150
// Validate configuration by checking for bucket existence on startup
56-
this.s3Bucket.headBucket(new HeadBucketRequest(bucketName))
151+
this.s3.headBucket(new HeadBucketRequest(bucketName))
152+
Logger.debug("Confirmed access to configured S3 bucket. S3ByteStorageService loading is complete.")
57153
} catch {
58-
case sdke @ (_: AmazonClientException | _: AmazonServiceException) => {
154+
case sdke@(_: AmazonClientException | _: AmazonServiceException) => {
59155
if (sdke.getMessage.contains("Status Code: 404")) {
60156
Logger.warn("Configured S3 bucket does not exist, attempting to create it now...")
61157
try {
62158
// Bucket does not exist - create the bucket
63-
this.s3Bucket.createBucket(new CreateBucketRequest(bucketName))
159+
this.s3.createBucket(new CreateBucketRequest(bucketName))
160+
Logger.debug("Created configured S3 bucket. S3ByteStorageService loading is complete.")
64161
} catch {
65162
// Bucket could not be created - abort
66163
case _: Throwable => throw new RuntimeException("Bad S3 configuration: Bucket does not exist and could not be created.")
@@ -75,50 +172,13 @@ class S3ByteStorageService @Inject()() extends ByteStorageService {
75172
throw new RuntimeException("Bad S3 configuration: an unknown error has occurred - " + errMsg)
76173
}
77174
}
78-
case _: Throwable => handleUnknownError(_)
175+
case _: Throwable => throw new RuntimeException("Bad S3 configuration: an unknown error has occurred.")
79176
}
80177
}
81178
case _ => throw new RuntimeException("Bad S3 configuration: verify that you have set all configuration options.")
82179
}
83180
}
84181

85-
/**
86-
* Grabs config parameters from Clowder to return a
87-
* AmazonS3 pointing at the configured service endpoint.
88-
*/
89-
def s3Bucket(): AmazonS3 = {
90-
(Play.current.configuration.getString(S3ByteStorageService.ServiceEndpoint),
91-
Play.current.configuration.getString(S3ByteStorageService.AccessKey),
92-
Play.current.configuration.getString(S3ByteStorageService.SecretKey)) match {
93-
case (Some(serviceEndpoint), Some(accessKey), Some(secretKey)) => {
94-
val credentials = new BasicAWSCredentials(accessKey, secretKey)
95-
val clientConfiguration = new ClientConfiguration
96-
clientConfiguration.setSignerOverride("AWSS3V4SignerType")
97-
98-
Logger.debug("Created S3 Client for " + serviceEndpoint)
99-
100-
val region = Play.current.configuration.getString(S3ByteStorageService.Region) match {
101-
case Some(region) => region
102-
case _ => Regions.US_EAST_1.name()
103-
}
104-
105-
return AmazonS3ClientBuilder.standard()
106-
// NOTE: Region is ignored for MinIO case?
107-
// TODO: Allow user to set region for AWS case?
108-
.withEndpointConfiguration(new AwsClientBuilder.EndpointConfiguration(serviceEndpoint, region))
109-
.withPathStyleAccessEnabled(true)
110-
.withClientConfiguration(clientConfiguration)
111-
.withCredentials(new AWSStaticCredentialsProvider(credentials))
112-
.build()
113-
}
114-
case _ => {
115-
val errMsg = "Bad S3 configuration: verify that you have set all configuration options."
116-
Logger.error(errMsg)
117-
throw new RuntimeException(errMsg)
118-
}
119-
}
120-
}
121-
122182
/**
123183
* Store bytes to the specified path within the configured S3 bucket.
124184
*
@@ -131,8 +191,8 @@ class S3ByteStorageService @Inject()() extends ByteStorageService {
131191
Play.current.configuration.getString(S3ByteStorageService.BucketName) match {
132192
case None => Logger.error("Failed saving bytes: failed to find configured S3 bucketName.")
133193
case Some(bucketName) => {
134-
val xferManager: TransferManager = TransferManagerBuilder.standard().withS3Client(this.s3Bucket).build
135194
try {
195+
val xferManager = TransferManagerBuilder.standard().withS3Client(this.s3).build
136196
Logger.debug("Saving file to: /" + bucketName + "/" + prefix)
137197

138198
val id = UUID.generate.stringify
@@ -163,7 +223,7 @@ class S3ByteStorageService @Inject()() extends ByteStorageService {
163223
//XferMgrProgress.showTransferProgress(xfer)
164224
// or block with Transfer.waitForCompletion()
165225
xfer.waitForCompletion()
166-
xferManager.shutdownNow()
226+
xferManager.shutdownNow(false)
167227

168228
Logger.debug("File saved to: /" + bucketName + "/" + prefix + "/" + targetPath)
169229

@@ -199,7 +259,7 @@ class S3ByteStorageService @Inject()() extends ByteStorageService {
199259
// Download object from S3 bucket
200260
// NOTE: path should already contain the prefix
201261
val rangeObjectRequest = new GetObjectRequest(bucketName, path)
202-
val objectPortion = this.s3Bucket.getObject(rangeObjectRequest)
262+
val objectPortion = s3.getObject(rangeObjectRequest)
203263

204264
return Option(objectPortion.getObjectContent)
205265
} catch {
@@ -231,7 +291,7 @@ class S3ByteStorageService @Inject()() extends ByteStorageService {
231291
try {
232292
// Delete object from S3 bucket
233293
// NOTE: path should already contain the prefix
234-
this.s3Bucket.deleteObject(bucketName, path)
294+
s3.deleteObject(bucketName, path)
235295
return true
236296
} catch {
237297
case ase: AmazonServiceException => handleASE(ase)
@@ -245,31 +305,4 @@ class S3ByteStorageService @Inject()() extends ByteStorageService {
245305
// Return false (in case of failure)
246306
return false
247307
}
248-
249-
/* Reusable handlers for various Exception types */
250-
def handleUnknownError(err: Exception = null) = {
251-
if (err != null) {
252-
Logger.error("An unknown error occurred in the S3ByteStorageService: " + err.toString)
253-
} else {
254-
Logger.error("An unknown error occurred in the S3ByteStorageService.")
255-
}
256-
}
257-
258-
def handleIOE(err: IOException) = {
259-
Logger.error("IOException occurred in the S3ByteStorageService: " + err)
260-
}
261-
262-
def handleACE(ace: AmazonClientException) = {
263-
Logger.error("Caught an AmazonClientException, which " + "means the client encountered " + "an internal error while trying to " + "communicate with S3, " + "such as not being able to access the network.")
264-
Logger.error("Error Message: " + ace.getMessage)
265-
}
266-
267-
def handleASE(ase: AmazonServiceException) = {
268-
Logger.error("Caught an AmazonServiceException, which " + "means your request made it " + "to Amazon S3, but was rejected with an error response" + " for some reason.")
269-
Logger.error("Error Message: " + ase.getMessage)
270-
Logger.error("HTTP Status Code: " + ase.getStatusCode)
271-
Logger.error("AWS Error Code: " + ase.getErrorCode)
272-
Logger.error("Error Type: " + ase.getErrorType)
273-
Logger.error("Request ID: " + ase.getRequestId)
274-
}
275308
}

0 commit comments

Comments
 (0)