From e8b3124b29471955dc267e1a6fabea88da701e68 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Thu, 7 May 2026 09:19:41 +0100 Subject: [PATCH 01/39] Disable lower environment. --- image-loader/app/model/Uploader.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/image-loader/app/model/Uploader.scala b/image-loader/app/model/Uploader.scala index af5acf651d..d11f95a190 100644 --- a/image-loader/app/model/Uploader.scala +++ b/image-loader/app/model/Uploader.scala @@ -472,6 +472,7 @@ class Uploader( )) // TODO: centralise where all these URLs are constructed } yield { + /* config.maybeLowerEnvironmentQueueBucketToSampleInto.foreach { lowerEnvironmentQueueBucket => if (math.random() < config.lowerEnvironmentSamplingPercentageAsDecimal) { val mediaId = imageUpload.image.id @@ -487,7 +488,7 @@ class Uploader( } } } - + */ UploadStatusUri(s"${config.rootUri(instance)}/uploadStatus/${uploadRequest.imageId}") } From 8a8cc9e6e252bb8ce8c9ce8641b44a0508b5a916 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 15 Dec 2024 15:00:53 +0000 Subject: [PATCH 02/39] Push raw s3 client usages to the S3 trait. S3 client field now private Protects usages from the specific of which client is used to access which bucket. --- .../com/gu/mediaservice/lib/BaseStore.scala | 5 +- .../lib/ImageIngestOperations.scala | 8 +-- .../gu/mediaservice/lib/S3ImageStorage.scala | 10 ++-- .../gu/mediaservice/lib/auth/KeyStore.scala | 5 +- .../com/gu/mediaservice/lib/aws/S3.scala | 60 ++++++++++++++++++- image-loader/app/lib/ImageLoaderStore.scala | 8 +-- thrall/app/controllers/ReaperController.scala | 14 +++-- 7 files changed, 81 insertions(+), 29 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/BaseStore.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/BaseStore.scala index 0aeb698ccc..2628508c7e 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/BaseStore.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/BaseStore.scala @@ -25,15 +25,14 @@ abstract class BaseStore[TStoreKey, TStoreVal](bucket: String, config: CommonCon protected def getS3Object(key: String): Option[String] = s3.getObjectAsString(bucket, key) protected def getLatestS3Stream: Option[InputStream] = { - val objects = s3.client - .listObjects(bucket).getObjectSummaries.asScala + val objects = s3.listObjects(bucket).getObjectSummaries.asScala .filterNot(_.getKey == "AMAZON_SES_SETUP_NOTIFICATION") if (objects.nonEmpty) { val obj = objects.maxBy(_.getLastModified) logger.info(s"Latest key ${obj.getKey} in bucket $bucket") - val stream = s3.client.getObject(bucket, obj.getKey).getObjectContent + val stream = s3.getObject(bucket, obj).getObjectContent Some(stream) } else { logger.error(s"Bucket $bucket is empty") diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala index 96c9412556..43ba653625 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala @@ -1,6 +1,6 @@ package com.gu.mediaservice.lib -import com.amazonaws.services.s3.model.{DeleteObjectsRequest, MultiObjectDeleteException} +import com.amazonaws.services.s3.model.MultiObjectDeleteException import java.io.File import com.gu.mediaservice.lib.config.CommonConfig @@ -62,9 +62,7 @@ class ImageIngestOperations(imageBucket: String, thumbnailBucket: String, config case _ => Future { try { logger.info(s"Creating S3 bulkDelete request for $bucket / keys: " + keys.mkString(",")) - client.deleteObjects( - new DeleteObjectsRequest(bucket).withKeys(keys: _*) - ) + deleteObjects(bucket, keys) keys.map { key => key -> true }.toMap @@ -87,7 +85,7 @@ class ImageIngestOperations(imageBucket: String, thumbnailBucket: String, config def deletePNGs(ids: Set[String])(implicit instance: Instance) = bulkDelete(imageBucket, ids.map(id => optimisedPngKeyFromId(id)).toList) def doesOriginalExist(id: String)(implicit instance: Instance): Boolean = - client.doesObjectExist(imageBucket, fileKeyFromId(id)) + doesObjectExist(imageBucket, fileKeyFromId(id)) private def instanceAwareOriginalImageKey(storableImage: StorableOriginalImage) = { fileKeyFromId(storableImage.id)(storableImage.instance) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/S3ImageStorage.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/S3ImageStorage.scala index 30b5d4b21c..1c7298a3d3 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/S3ImageStorage.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/S3ImageStorage.scala @@ -28,20 +28,20 @@ class S3ImageStorage(config: CommonConfig) extends S3(config) with ImageStorage } def deleteImage(bucket: String, id: String)(implicit logMarker: LogMarker) = Future { - client.deleteObject(bucket, id) + deleteObject(bucket, id) logger.info(logMarker, s"Deleted image $id from bucket $bucket") } def deleteVersionedImage(bucket: String, id: String)(implicit logMarker: LogMarker) = Future { - val objectVersion = client.getObjectMetadata(bucket, id).getVersionId - client.deleteVersion(bucket, id, objectVersion) + val objectVersion = getObjectMetadata(bucket, id).getVersionId + deleteVersion(bucket, id, objectVersion) logger.info(logMarker, s"Deleted image $id from bucket $bucket (version: $objectVersion)") } def deleteFolder(bucket: String, id: String)(implicit logMarker: LogMarker) = Future { - val files = client.listObjects(bucket, id).getObjectSummaries.asScala + val files = listObjects(bucket, id).getObjectSummaries.asScala logger.info(s"Found ${files.size} files to delete in folder $id") - files.foreach(file => client.deleteObject(bucket, file.getKey)) + files.foreach(file => deleteObject(bucket, file.getKey)) logger.info(logMarker, s"Deleting images in folder $id from bucket $bucket") } diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/auth/KeyStore.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/auth/KeyStore.scala index bbb9da0ca1..ba98bcd177 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/auth/KeyStore.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/auth/KeyStore.scala @@ -4,7 +4,6 @@ import com.gu.mediaservice.lib.BaseStore import com.gu.mediaservice.lib.config.CommonConfig import com.gu.mediaservice.model.Instance -import scala.jdk.CollectionConverters._ import scala.concurrent.ExecutionContext class KeyStore(bucket: String, config: CommonConfig)(implicit ec: ExecutionContext) @@ -17,7 +16,7 @@ class KeyStore(bucket: String, config: CommonConfig)(implicit ec: ExecutionConte } private def fetchAll: Map[String, ApiAccessor] = { - val keys = s3.client.listObjects(bucket).getObjectSummaries.asScala.map(_.getKey) - keys.flatMap(k => getS3Object(k).map(k -> ApiAccessor(_))).toMap + s3.listObjectKeys(bucket).flatMap(k => getS3Object(k).map(k -> ApiAccessor(_))).toMap } + } diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala index 816c644f95..8963847e1b 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala @@ -7,12 +7,12 @@ import com.amazonaws.{AmazonServiceException, ClientConfiguration} import com.gu.mediaservice.lib.config.CommonConfig import com.gu.mediaservice.lib.logging.{GridLogging, LogMarker, Stopwatch} import com.gu.mediaservice.model._ -import org.joda.time.{DateTime, Duration} +import org.joda.time.DateTime import java.io.File import java.net.{URI, URL} -import scala.jdk.CollectionConverters._ import scala.concurrent.{ExecutionContext, Future} +import scala.jdk.CollectionConverters._ case class S3Object(uri: URI, size: Long, metadata: S3Metadata) @@ -65,7 +65,7 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with type Key = String type UserMetadata = Map[String, String] - lazy val client: AmazonS3 = S3Ops.buildS3Client(config) + private lazy val client: AmazonS3 = S3Ops.buildS3Client(config) def signUrl(bucket: Bucket, url: URI, image: Image, expiration: DateTime = cachableExpiration(), imageType: ImageFileType = Source): String = { // get path and remove leading `/` @@ -87,12 +87,46 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with client.generatePresignedUrl(request) } + def copyObject(sourceBucket: Bucket, destinationBucket: Bucket, key: String): CopyObjectResult = { + client.copyObject(sourceBucket, key, destinationBucket, key) + } + + def generatePresignedRequest(request: GeneratePresignedUrlRequest): URL = { + client.generatePresignedUrl(request) + } + + def deleteObject(bucket: Bucket, key: String): Unit = { + client.deleteObject(bucket, key) + } + + def deleteObjects(bucket: Bucket, keys: Seq[Bucket]): DeleteObjectsResult = { + client.deleteObjects( + new DeleteObjectsRequest(bucket).withKeys(keys: _*) + ) + } + + def deleteVersion(bucket: Bucket, id: Bucket, objectVersion: Bucket): Unit = { + client.deleteVersion(bucket, id, objectVersion) + } + + def doesObjectExist(bucket: Bucket, key: String) = { + client.doesObjectExist(bucket, key) + } + def getObject(bucket: Bucket, url: URI): model.S3Object = { // get path and remove leading `/` val key: Key = url.getPath.drop(1) client.getObject(new GetObjectRequest(bucket, key)) } + def getObject(bucket: Bucket, key: String): model.S3Object = { + client.getObject(new GetObjectRequest(bucket, key)) + } + + def getObject(bucket: Bucket, obj: S3ObjectSummary): model.S3Object = { + client.getObject(bucket, obj.getKey) + } + def getObjectAsString(bucket: Bucket, key: String): Option[String] = { val content = client.getObject(new GetObjectRequest(bucket, key)) val stream = content.getObjectContent @@ -108,6 +142,26 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with } } + def getObjectMetadata(bucket: Bucket, id: Bucket): ObjectMetadata = { + client.getObjectMetadata(bucket, id) + } + + def listObjects(bucket: String): ObjectListing = { + client.listObjects(bucket) + } + + def listObjects(bucket: String, prefix: String): ObjectListing = { + client.listObjects(bucket, prefix) + } + + def listObjectKeys(bucket: String): Seq[String] = { + client.listObjects(bucket).getObjectSummaries.asScala.map(_.getKey).toSeq + } + + def putObject(bucket: String, key: String, content: String): Unit = { + client.putObject(bucket, key, content) + } + def store(bucket: Bucket, id: Key, file: File, mimeType: Option[MimeType], meta: UserMetadata = Map.empty, cacheControl: Option[String] = None) (implicit ex: ExecutionContext, logMarker: LogMarker): Future[S3Object] = Future { diff --git a/image-loader/app/lib/ImageLoaderStore.scala b/image-loader/app/lib/ImageLoaderStore.scala index e2355ff0b0..5c41504a7f 100644 --- a/image-loader/app/lib/ImageLoaderStore.scala +++ b/image-loader/app/lib/ImageLoaderStore.scala @@ -31,7 +31,7 @@ class ImageLoaderStore(config: ImageLoaderConfig) extends lib.ImageIngestOperati } def getS3Object(key: String)(implicit logMarker: LogMarker): S3Object = handleNotFound(key) { - client.getObject(config.maybeIngestBucket.get, key) + getObject(config.maybeIngestBucket.get, key) } { logger.error(logMarker, s"Attempted to read $key from ingest bucket, but it does not exist.") } @@ -57,18 +57,18 @@ class ImageLoaderStore(config: ImageLoaderConfig) extends lib.ImageIngestOperati // sent by the client in manager.js request.putCustomRequestHeader("x-amz-meta-media-id", mediaId) - client.generatePresignedUrl(request).toString + generatePresignedRequest(request).toString } def moveObjectToFailedBucket(key: String)(implicit logMarker: LogMarker) = handleNotFound(key){ - client.copyObject(config.maybeIngestBucket.get, key, config.maybeFailBucket.get, key) // TODO Naked get - make optional + copyObject(config.maybeIngestBucket.get, config.maybeFailBucket.get, key) // TODO Naked get - make optional deleteObjectFromIngestBucket(key) } { logger.warn(logMarker, s"Attempted to copy $key from ingest bucket to fail bucket, but it does not exist.") } def deleteObjectFromIngestBucket(key: String)(implicit logMarker: LogMarker) = handleNotFound(key) { - client.deleteObject(config.maybeIngestBucket.get,key) + deleteObject(config.maybeIngestBucket.get, key) } { logger.warn(logMarker, s"Attempted to delete $key from ingest bucket, but it does not exist.") } diff --git a/thrall/app/controllers/ReaperController.scala b/thrall/app/controllers/ReaperController.scala index fb8a121b42..e0a615fd82 100644 --- a/thrall/app/controllers/ReaperController.scala +++ b/thrall/app/controllers/ReaperController.scala @@ -127,7 +127,7 @@ class ReaperController( case Some(reaperBucket) => val now = DateTime.now(DateTimeZone.UTC) val key = s"$deleteType/${s3DirNameFromDate(now)}/$deleteType-${now.toString()}.json" - store.client.putObject(reaperBucket, key, json.toString()) + store.putObject(reaperBucket, key, json.toString()) json } } @@ -213,8 +213,8 @@ class ReaperController( case (Some(reaperBucket), Some(countOfImagesToReap)) => val recentRecords = List(now, now.minusDays(1), now.minusDays(2)).flatMap { day => val s3DirName = s3DirNameFromDate(day) - store.client.listObjects(reaperBucket, s"soft/$s3DirName/").getObjectSummaries.asScala.toList ++ - store.client.listObjects(reaperBucket, s"hard/$s3DirName/").getObjectSummaries.asScala.toList + store.listObjects(reaperBucket, s"soft/$s3DirName/").getObjectSummaries.asScala.toList ++ + store.listObjects(reaperBucket, s"hard/$s3DirName/").getObjectSummaries.asScala.toList } val recentRecordKeys = recentRecords @@ -229,9 +229,11 @@ class ReaperController( def reaperRecord(key: String) = auth { config.maybeReaperBucket match { case None => NotImplemented("Reaper bucket not configured") case Some(reaperBucket) => - Ok( - store.client.getObjectAsString(reaperBucket, key) - ).as(JSON) + store.getObjectAsString(reaperBucket, key).map { record => + Ok(record).as(JSON) + }.getOrElse{ + NotFound + } }} def conf() = Action.async { From 751081506cd896a5c9da2742251dbcc7fe17178e Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Fri, 29 Nov 2024 16:04:26 +0000 Subject: [PATCH 03/39] Push s3 end point to all s3 users. Bucket specific end points. Pull image and thumbnail buckets to common config. --- .../com/gu/mediaservice/lib/BaseStore.scala | 4 ++- .../lib/ImageIngestOperations.scala | 23 ++++++++------- .../lib/ImageQuarantineOperations.scala | 4 +-- .../gu/mediaservice/lib/ImageStorage.scala | 2 +- .../gu/mediaservice/lib/S3ImageStorage.scala | 6 ++-- .../gu/mediaservice/lib/auth/KeyStore.scala | 4 +-- .../com/gu/mediaservice/lib/aws/S3.scala | 29 ++++++++++--------- .../lib/config/CommonConfig.scala | 8 ++++- .../src/test/resources/application.conf | 4 +++ cropper/app/lib/CropStore.scala | 4 +-- cropper/app/lib/CropperConfig.scala | 3 +- image-loader/app/lib/ImageLoaderConfig.scala | 5 ++-- image-loader/app/lib/ImageLoaderStore.scala | 3 +- image-loader/app/lib/QuarantineStore.scala | 2 +- image-loader/app/model/Projector.scala | 6 ++-- image-loader/app/model/Uploader.scala | 8 +++-- .../test/scala/model/ImageUploadTest.scala | 6 ++-- .../test/scala/model/ProjectorTest.scala | 3 +- media-api/app/lib/ImageResponse.scala | 2 +- media-api/app/lib/MediaApiConfig.scala | 8 ++--- media-api/app/lib/QuotaStore.scala | 5 ++-- media-api/app/lib/UsageQuota.scala | 6 ++-- media-api/app/lib/UsageStore.scala | 5 ++-- .../ApiKeyAuthenticationProvider.scala | 2 +- .../ApiKeyAuthenticationProviderTest.scala | 2 +- .../lib/JsonValueCodecJsValue.scala | 1 + thrall/app/lib/ThrallConfig.scala | 4 --- thrall/app/lib/ThrallStore.scala | 2 +- 28 files changed, 91 insertions(+), 70 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/BaseStore.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/BaseStore.scala index 2628508c7e..31d3cbb610 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/BaseStore.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/BaseStore.scala @@ -14,9 +14,11 @@ import scala.concurrent.duration._ import scala.util.control.NonFatal -abstract class BaseStore[TStoreKey, TStoreVal](bucket: String, config: CommonConfig)(implicit ec: ExecutionContext) +abstract class BaseStore[TStoreKey, TStoreVal](bucket: String, config: CommonConfig, s3Endpoint: String)(implicit ec: ExecutionContext) extends GridLogging { + def s3Endpoint: String + val s3 = new S3(config) protected val store: AtomicReference[Map[TStoreKey, TStoreVal]] = new AtomicReference(Map.empty) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala index 43ba653625..895e4161bb 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala @@ -21,7 +21,7 @@ object ImageIngestOperations { private def snippetForId(id: String) = id.take(6).mkString("/") + "/" + id } -class ImageIngestOperations(imageBucket: String, thumbnailBucket: String, config: CommonConfig, isVersionedS3: Boolean = false) +class ImageIngestOperations(imageBucket: String, thumbnailBucket: String, config: CommonConfig, isVersionedS3: Boolean = false, imageBucketS3Endpoint: String, thumbnailBucketS3Endpoint: String) extends S3ImageStorage(config) with StrictLogging { import ImageIngestOperations.{fileKeyFromId, optimisedPngKeyFromId} @@ -38,7 +38,7 @@ class ImageIngestOperations(imageBucket: String, thumbnailBucket: String, config val instanceSpecificKey = instanceAwareOriginalImageKey(storableImage) logger.info(s"Storing original image to instance specific key:$imageBucket / $instanceSpecificKey") storeImage(imageBucket, instanceSpecificKey, storableImage.file, Some(storableImage.mimeType), - storableImage.meta, overwrite = false) + storableImage.meta, overwrite = false, s3Endpoint = imageBucketS3Endpoint) } private def storeThumbnailImage(storableImage: StorableThumbImage) @@ -46,7 +46,7 @@ class ImageIngestOperations(imageBucket: String, thumbnailBucket: String, config val instanceSpecificKey = instanceAwareThumbnailImageKey(storableImage) logger.info(s"Storing thumbnail to instance specific key: $thumbnailBucket / $instanceSpecificKey") storeImage(thumbnailBucket, instanceSpecificKey, storableImage.file, Some(storableImage.mimeType), - overwrite = true) + overwrite = true, s3Endpoint = thumbnailBucketS3Endpoint) } private def storeOptimisedImage(storableImage: StorableOptimisedImage) @@ -54,7 +54,7 @@ class ImageIngestOperations(imageBucket: String, thumbnailBucket: String, config val instanceSpecificKey = optimisedPngKeyFromId(storableImage.id)(storableImage.instance) logger.info(s"Storing optimised image to instance specific key: $thumbnailBucket / $instanceSpecificKey") storeImage(imageBucket, instanceSpecificKey, storableImage.file, Some(storableImage.mimeType), - overwrite = true) + overwrite = true, s3Endpoint = imageBucketS3Endpoint) } private def bulkDelete(bucket: String, keys: List[String]): Future[Map[String, Boolean]] = keys match { @@ -105,35 +105,38 @@ sealed trait ImageWrapper { val instance: Instance } sealed trait StorableImage extends ImageWrapper { - def toProjectedS3Object(thumbBucket: String): S3Object = S3Object( + def toProjectedS3Object(thumbBucket: String, thumbBucketEndpoint: String): S3Object = S3Object( thumbBucket, ImageIngestOperations.fileKeyFromId(id)(instance), file, Some(mimeType), lastModified = None, - meta + meta, + s3Endpoint = thumbBucketEndpoint ) } case class StorableThumbImage(id: String, file: File, mimeType: MimeType, meta: Map[String, String] = Map.empty, instance: Instance) extends StorableImage case class StorableOriginalImage(id: String, file: File, mimeType: MimeType, lastModified: DateTime, meta: Map[String, String] = Map.empty, instance: Instance) extends StorableImage { - override def toProjectedS3Object(thumbBucket: String): S3Object = S3Object( + override def toProjectedS3Object(thumbBucket: String, thumbBucketEndpoint: String): S3Object = S3Object( thumbBucket, ImageIngestOperations.fileKeyFromId(id)(instance), file, Some(mimeType), lastModified = Some(lastModified), - meta + meta, + s3Endpoint = thumbBucketEndpoint ) } case class StorableOptimisedImage(id: String, file: File, mimeType: MimeType, meta: Map[String, String] = Map.empty, instance: Instance) extends StorableImage { - override def toProjectedS3Object(thumbBucket: String): S3Object = S3Object( + override def toProjectedS3Object(thumbBucket: String, thumbBucketEndpoint: String): S3Object = S3Object( thumbBucket, ImageIngestOperations.optimisedPngKeyFromId(id)(instance), file, Some(mimeType), lastModified = None, - meta = meta + meta = meta, + s3Endpoint = thumbBucketEndpoint ) } diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageQuarantineOperations.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageQuarantineOperations.scala index 0cc3a146a0..51b04e980d 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageQuarantineOperations.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageQuarantineOperations.scala @@ -8,12 +8,12 @@ import com.gu.mediaservice.model.{Instance, MimeType} import scala.concurrent.Future -class ImageQuarantineOperations(quarantineBucket: String, config: CommonConfig, isVersionedS3: Boolean = false) +class ImageQuarantineOperations(quarantineBucket: String, config: CommonConfig, isVersionedS3: Boolean = false, s3Endpoint: String) extends S3ImageStorage(config) { def storeQuarantineImage(id: String, file: File, mimeType: Option[MimeType], meta: Map[String, String] = Map.empty) (implicit logMarker: LogMarker, instance: Instance): Future[S3Object] = - storeImage(quarantineBucket, ImageIngestOperations.fileKeyFromId(id), file, mimeType, meta, overwrite = true) + storeImage(quarantineBucket, ImageIngestOperations.fileKeyFromId(id), file, mimeType, meta, overwrite = true, s3Endpoint = s3Endpoint) } diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageStorage.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageStorage.scala index 1e57a3513a..fb41afcdfd 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageStorage.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageStorage.scala @@ -38,7 +38,7 @@ trait ImageStorage { * The file can safely be deleted afterwards. */ def storeImage(bucket: String, id: String, file: File, mimeType: Option[MimeType], - meta: Map[String, String] = Map.empty, overwrite: Boolean) + meta: Map[String, String] = Map.empty, overwrite: Boolean, s3Endpoint: String) (implicit logMarker: LogMarker): Future[S3Object] def deleteImage(bucket: String, id: String)(implicit logMarker: LogMarker): Future[Unit] diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/S3ImageStorage.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/S3ImageStorage.scala index 1c7298a3d3..919e688c1c 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/S3ImageStorage.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/S3ImageStorage.scala @@ -15,13 +15,13 @@ class S3ImageStorage(config: CommonConfig) extends S3(config) with ImageStorage private val cacheSetting = Some(cacheForever) def storeImage(bucket: String, id: String, file: File, mimeType: Option[MimeType], - meta: Map[String, String] = Map.empty, overwrite: Boolean) + meta: Map[String, String] = Map.empty, overwrite: Boolean, s3Endpoint: String) (implicit logMarker: LogMarker) = { logger.info(logMarker, s"bucket: $bucket, id: $id, meta: $meta") val eventualObject = if (overwrite) { - store(bucket, id, file, mimeType, meta, cacheSetting) + store(bucket, id, file, mimeType, meta, cacheSetting, s3Endpoint) } else { - storeIfNotPresent(bucket, id, file, mimeType, meta, cacheSetting) + storeIfNotPresent(bucket, id, file, mimeType, meta, cacheSetting, s3Endpoint) } eventualObject.onComplete(o => logger.info(logMarker, s"storeImage completed $o")) eventualObject diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/auth/KeyStore.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/auth/KeyStore.scala index ba98bcd177..0944203002 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/auth/KeyStore.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/auth/KeyStore.scala @@ -6,8 +6,8 @@ import com.gu.mediaservice.model.Instance import scala.concurrent.ExecutionContext -class KeyStore(bucket: String, config: CommonConfig)(implicit ec: ExecutionContext) - extends BaseStore[String, ApiAccessor](bucket, config)(ec) { +class KeyStore(bucket: String, config: CommonConfig, val s3Endpoint: String)(implicit ec: ExecutionContext) + extends BaseStore[String, ApiAccessor](bucket, config, s3Endpoint)(ec) { def lookupIdentity(key: String)(implicit instance: Instance): Option[ApiAccessor] = store.get().get(instance.id + "/" + key) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala index 8963847e1b..1f95271e7c 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala @@ -17,16 +17,16 @@ import scala.jdk.CollectionConverters._ case class S3Object(uri: URI, size: Long, metadata: S3Metadata) object S3Object { - def objectUrl(bucket: String, key: String): URI = { - val bucketUrl = s"$bucket.${S3Ops.s3Endpoint}" + private def objectUrl(bucket: String, key: String, s3Endpoint: String): URI = { + val bucketUrl = s"$bucket.$s3Endpoint" new URI("http", bucketUrl, s"/$key", null) } - def apply(bucket: String, key: String, size: Long, metadata: S3Metadata): S3Object = - apply(objectUrl(bucket, key), size, metadata) + def apply(bucket: String, key: String, size: Long, metadata: S3Metadata, s3Endpoint: String): S3Object = + apply(objectUrl(bucket, key, s3Endpoint), size, metadata) def apply(bucket: String, key: String, file: File, mimeType: Option[MimeType], lastModified: Option[DateTime], - meta: Map[String, String] = Map.empty, cacheControl: Option[String] = None): S3Object = { + meta: Map[String, String] = Map.empty, cacheControl: Option[String] = None, s3Endpoint: String): S3Object = { S3Object( bucket, key, @@ -38,7 +38,8 @@ object S3Object { cacheControl, lastModified ) - ) + ), + s3Endpoint ) } } @@ -113,7 +114,7 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with client.doesObjectExist(bucket, key) } - def getObject(bucket: Bucket, url: URI): model.S3Object = { + def getObject(bucket: Bucket, url: URI): model.S3Object = { // TODO why can't this just be by bucket + key to remove end point knowledge // get path and remove leading `/` val key: Key = url.getPath.drop(1) client.getObject(new GetObjectRequest(bucket, key)) @@ -162,7 +163,7 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with client.putObject(bucket, key, content) } - def store(bucket: Bucket, id: Key, file: File, mimeType: Option[MimeType], meta: UserMetadata = Map.empty, cacheControl: Option[String] = None) + def store(bucket: Bucket, id: Key, file: File, mimeType: Option[MimeType], meta: UserMetadata = Map.empty, cacheControl: Option[String] = None, s3Endpoint: String) (implicit ex: ExecutionContext, logMarker: LogMarker): Future[S3Object] = Future { val metadata = new ObjectMetadata @@ -182,11 +183,11 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with client.putObject(req) // once we've completed the PUT read back to ensure that we are returning reality val metadata = client.getObjectMetadata(bucket, id) - S3Object(bucket, id, metadata.getContentLength, S3Metadata(metadata)) + S3Object(bucket, id, metadata.getContentLength, S3Metadata(metadata), s3Endpoint) }(markers) } - def storeIfNotPresent(bucket: Bucket, id: Key, file: File, mimeType: Option[MimeType], meta: UserMetadata = Map.empty, cacheControl: Option[String] = None) + def storeIfNotPresent(bucket: Bucket, id: Key, file: File, mimeType: Option[MimeType], meta: UserMetadata = Map.empty, cacheControl: Option[String] = None, s3Endpoint: String) (implicit ex: ExecutionContext, logMarker: LogMarker): Future[S3Object] = { Future{ Some(client.getObjectMetadata(bucket, id)) @@ -196,13 +197,13 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with }.flatMap { case Some(objectMetadata) => logger.info(logMarker, s"Skipping storing of S3 file $id as key is already present in bucket $bucket") - Future.successful(S3Object(bucket, id, objectMetadata.getContentLength, S3Metadata(objectMetadata))) + Future.successful(S3Object(bucket, id, objectMetadata.getContentLength, S3Metadata(objectMetadata), s3Endpoint)) case None => - store(bucket, id, file, mimeType, meta, cacheControl) + store(bucket, id, file, mimeType, meta, cacheControl, s3Endpoint) } } - def list(bucket: Bucket, prefixDir: String) + def list(bucket: Bucket, prefixDir: String, s3Endpoint: String) (implicit ex: ExecutionContext): Future[List[S3Object]] = Future { val req = new ListObjectsRequest().withBucketName(bucket).withPrefix(s"$prefixDir/") @@ -210,7 +211,7 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with val summaries = listing.getObjectSummaries.asScala summaries.map(summary => (summary.getKey, summary)).foldLeft(List[S3Object]()) { case (memo: List[S3Object], (key: String, summary: S3ObjectSummary)) => - S3Object(bucket, key, summary.getSize, getMetadata(bucket, key)) :: memo + S3Object(bucket, key, summary.getSize, getMetadata(bucket, key), s3Endpoint) :: memo } } diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala index 83539c3fc0..1220268f8c 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala @@ -1,6 +1,6 @@ package com.gu.mediaservice.lib.config -import com.gu.mediaservice.lib.aws.{AwsClientV1BuilderUtils, AwsClientV2BuilderUtils, KinesisSenderConfig} +import com.gu.mediaservice.lib.aws.{AwsClientV1BuilderUtils, AwsClientV2BuilderUtils, KinesisSenderConfig, S3Ops} import com.gu.mediaservice.model.UsageRightsSpec import com.typesafe.config.Config import com.typesafe.scalalogging.StrictLogging @@ -55,6 +55,7 @@ abstract class CommonConfig(resources: GridConfigResources) extends AwsClientV1B val maybeIngestSqsQueueUrl: Option[String] = stringOpt("sqs.ingest.queue.url") val maybeIngestBucket: Option[String] = stringOpt("s3.ingest.bucket") + val maybeIngestBucketEndpoint: Option[String] = stringOpt("s3.ingest.bucket").map(_ => S3Ops.s3Endpoint) val maybeFailBucket: Option[String] = stringOpt("s3.fail.bucket") val maybeQuarantineBucket: Option[String] = stringOpt("s3.quarantine.bucket") @@ -74,6 +75,11 @@ abstract class CommonConfig(resources: GridConfigResources) extends AwsClientV1B val services = new SingleHostServices(domainRoot) + val imageBucket: String = string("s3.image.bucket") + val imageBucketS3Endpoint: String = "s3.amazonaws.com" + val thumbnailBucket: String = string("s3.thumb.bucket") + val thumbnailBucketS3Endpoint: String = "s3.amazonaws.com" + /** * Load in a list of domain metadata specifications from configuration. For example: * {{{ diff --git a/common-lib/src/test/resources/application.conf b/common-lib/src/test/resources/application.conf index 05d1db98e6..e3724393da 100644 --- a/common-lib/src/test/resources/application.conf +++ b/common-lib/src/test/resources/application.conf @@ -61,3 +61,7 @@ instance.service.my="" instance.service.instances="" usageEvents.queue.name="" + +s3.image.bucket="images" + +s3.thumb.bucket="thumbs" diff --git a/cropper/app/lib/CropStore.scala b/cropper/app/lib/CropStore.scala index 1cc815d2cb..cf6eb37e6c 100644 --- a/cropper/app/lib/CropStore.scala +++ b/cropper/app/lib/CropStore.scala @@ -15,7 +15,7 @@ class CropStore(config: CropperConfig) extends S3ImageStorage(config) with CropS def storeCropSizing(file: File, filename: String, mimeType: MimeType, crop: Crop, dimensions: Dimensions)(implicit logMarker: LogMarker): Future[Asset] = { val metadata = metadataForCrop(crop, dimensions) - storeImage(config.imgPublishingBucket, filename, file, Some(mimeType), metadata, overwrite = true) map { s3Object => + storeImage(config.imgPublishingBucket, filename, file, Some(mimeType), metadata, overwrite = true, config.imgPublishingBucketS3Endpoint) map { s3Object => Asset( translateImgHost(s3Object.uri), Some(s3Object.size), @@ -27,7 +27,7 @@ class CropStore(config: CropperConfig) extends S3ImageStorage(config) with CropS } def listCrops(id: String, instance: Instance): Future[List[Crop]] = { - list(config.imgPublishingBucket, folderForImagesCrops(id, instance)).map { crops => // TODO crops layout want to be pull up + list(config.imgPublishingBucket, folderForImagesCrops(id, instance), config.imgPublishingBucketS3Endpoint).map { crops => // TODO crops layout want to be pull up crops.foldLeft(Map[String, Crop]()) { case (map, (s3Object)) => { val filename::containingFolder::_ = s3Object.uri.getPath.split("/").reverse.toList diff --git a/cropper/app/lib/CropperConfig.scala b/cropper/app/lib/CropperConfig.scala index 96d043d60a..8aebea551e 100644 --- a/cropper/app/lib/CropperConfig.scala +++ b/cropper/app/lib/CropperConfig.scala @@ -7,9 +7,8 @@ import java.io.File class CropperConfig(resources: GridConfigResources) extends CommonConfig(resources) { - val imageBucket: String = string("s3.image.bucket") - val imgPublishingBucket = string("publishing.image.bucket") + val imgPublishingBucketS3Endpoint: String = "s3.amazonaws.com" val canDownloadCrop: Boolean = boolean("canDownloadCrop") diff --git a/image-loader/app/lib/ImageLoaderConfig.scala b/image-loader/app/lib/ImageLoaderConfig.scala index 37d43aa078..7a17360631 100644 --- a/image-loader/app/lib/ImageLoaderConfig.scala +++ b/image-loader/app/lib/ImageLoaderConfig.scala @@ -10,11 +10,12 @@ import play.api.inject.ApplicationLifecycle import scala.concurrent.duration.FiniteDuration class ImageLoaderConfig(resources: GridConfigResources) extends CommonConfig(resources) with StrictLogging { - val imageBucket: String = string("s3.image.bucket") val maybeImageReplicaBucket: Option[String] = stringOpt("s3.image.replicaBucket") - val thumbnailBucket: String = string("s3.thumb.bucket") + val quarantineBucket: Option[String] = stringOpt("s3.quarantine.bucket") + val quarantineBucketS3Endpoint: String = "s3.amazonaws.com" + val uploadToQuarantineEnabled: Boolean = boolean("upload.quarantine.enabled") val lowerEnvironmentSamplingPercentageAsDecimal = intOpt("s3.sampling.percentage").getOrElse(1) / 100.0 val maybeLowerEnvironmentQueueBucketToSampleInto = stringOpt("s3.sampling.targetBucket") diff --git a/image-loader/app/lib/ImageLoaderStore.scala b/image-loader/app/lib/ImageLoaderStore.scala index 5c41504a7f..fe5f0a0286 100644 --- a/image-loader/app/lib/ImageLoaderStore.scala +++ b/image-loader/app/lib/ImageLoaderStore.scala @@ -15,7 +15,7 @@ import scala.concurrent.Future class S3FileDoesNotExistException extends Exception() -class ImageLoaderStore(config: ImageLoaderConfig) extends lib.ImageIngestOperations(config.imageBucket, config.thumbnailBucket, config) with GridLogging { +class ImageLoaderStore(config: ImageLoaderConfig) extends lib.ImageIngestOperations(config.imageBucket, config.thumbnailBucket, config, imageBucketS3Endpoint = config.imageBucketS3Endpoint, thumbnailBucketS3Endpoint = config.thumbnailBucketS3Endpoint) with GridLogging { private def handleNotFound[T](key: String)(doWork: => T)(loggingIfNotFound: => Unit): T = { try { @@ -43,6 +43,7 @@ class ImageLoaderStore(config: ImageLoaderConfig) extends lib.ImageIngestOperati file, mimeType = None, // we don't care as this is just the queue bucket meta = s3Meta, + s3Endpoint = config.maybeIngestBucketEndpoint.get ) } diff --git a/image-loader/app/lib/QuarantineStore.scala b/image-loader/app/lib/QuarantineStore.scala index 4fc8581672..e73e28ead2 100644 --- a/image-loader/app/lib/QuarantineStore.scala +++ b/image-loader/app/lib/QuarantineStore.scala @@ -3,4 +3,4 @@ package lib.storage import lib.ImageLoaderConfig import com.gu.mediaservice.lib -class QuarantineStore(config: ImageLoaderConfig) extends lib.ImageQuarantineOperations(config.maybeQuarantineBucket.get, config) +class QuarantineStore(config: ImageLoaderConfig) extends lib.ImageQuarantineOperations(config.quarantineBucket.get, config, s3Endpoint = config.quarantineBucketS3Endpoint) diff --git a/image-loader/app/model/Projector.scala b/image-loader/app/model/Projector.scala index d1d5ac1e30..91ba54362a 100644 --- a/image-loader/app/model/Projector.scala +++ b/image-loader/app/model/Projector.scala @@ -188,13 +188,13 @@ class ImageUploadProjectionOps(config: ImageUploadOpsCfg, } private def projectOriginalFileAsS3Model(storableOriginalImage: StorableOriginalImage) = - Future.successful(storableOriginalImage.toProjectedS3Object(config.originalFileBucket)) + Future.successful(storableOriginalImage.toProjectedS3Object(config.originalFileBucket, config.originalFileBucketEndpoint)) private def projectThumbnailFileAsS3Model(storableThumbImage: StorableThumbImage) = - Future.successful(storableThumbImage.toProjectedS3Object(config.thumbBucket)) + Future.successful(storableThumbImage.toProjectedS3Object(config.thumbBucket, config.thumbBucketEndpoint)) private def projectOptimisedPNGFileAsS3Model(storableOptimisedImage: StorableOptimisedImage) = - Future.successful(storableOptimisedImage.toProjectedS3Object(config.originalFileBucket)) + Future.successful(storableOptimisedImage.toProjectedS3Object(config.originalFileBucket, config.originalFileBucketEndpoint)) private def fetchThumbFile( imageId: String, outFile: File, instance: Instance)(implicit ec: ExecutionContext, logMarker: LogMarker): Future[Option[(File, MimeType)]] = { diff --git a/image-loader/app/model/Uploader.scala b/image-loader/app/model/Uploader.scala index d11f95a190..dd347380d5 100644 --- a/image-loader/app/model/Uploader.scala +++ b/image-loader/app/model/Uploader.scala @@ -67,7 +67,9 @@ case class ImageUploadOpsCfg( thumbQuality: Double, transcodedMimeTypes: List[MimeType], originalFileBucket: String, - thumbBucket: String + originalFileBucketEndpoint: String, + thumbBucket: String, + thumbBucketEndpoint: String ) case class ImageUploadOpsDependencies( @@ -94,7 +96,9 @@ object Uploader extends GridLogging { config.thumbQuality, config.transcodedMimeTypes, config.imageBucket, - config.thumbnailBucket + config.imageBucketS3Endpoint, + config.thumbnailBucket, + config.thumbnailBucketS3Endpoint ) } diff --git a/image-loader/test/scala/model/ImageUploadTest.scala b/image-loader/test/scala/model/ImageUploadTest.scala index 9711519a0c..cea1d8cedb 100644 --- a/image-loader/test/scala/model/ImageUploadTest.scala +++ b/image-loader/test/scala/model/ImageUploadTest.scala @@ -1,7 +1,7 @@ package model import com.drew.imaging.ImageProcessingException -import com.gu.mediaservice.lib.aws.{S3Metadata, S3Object, S3ObjectMetadata} +import com.gu.mediaservice.lib.aws.{S3Metadata, S3Object, S3ObjectMetadata, S3Ops} import com.gu.mediaservice.lib.cleanup.ImageProcessor import com.gu.mediaservice.lib.imaging.ImageOperations import com.gu.mediaservice.lib.logging.LogMarker @@ -32,7 +32,7 @@ class ImageUploadTest extends AsyncFunSuite with Matchers with MockitoSugar { private implicit val logMarker: MockLogMarker = new MockLogMarker() // For mime type info, see https://github.com/guardian/grid/pull/2568 val tempDir = new File("/tmp") - val mockConfig: ImageUploadOpsCfg = ImageUploadOpsCfg(tempDir, 256, 85d, List(Tiff), "img-bucket", "thumb-bucket") + val mockConfig: ImageUploadOpsCfg = ImageUploadOpsCfg(tempDir, 256, 85d, List(Tiff), "img-bucket", S3Ops.s3Endpoint, "thumb-bucket", S3Ops.s3Endpoint) /** * @todo: I flailed about until I found a path that worked, but @@ -53,7 +53,7 @@ class ImageUploadTest extends AsyncFunSuite with Matchers with MockitoSugar { def mockStore = (a: StorableImage) => Future.successful( - S3Object("madeupname", "madeupkey", a.file, Some(a.mimeType), None, a.meta, None) + S3Object("madeupname", "madeupkey", a.file, Some(a.mimeType), None, a.meta, None, S3Ops.s3Endpoint) ) def storeOrProjectOriginalFile: StorableOriginalImage => Future[S3Object] = mockStore diff --git a/image-loader/test/scala/model/ProjectorTest.scala b/image-loader/test/scala/model/ProjectorTest.scala index 3c5d384863..0d8f565ff4 100644 --- a/image-loader/test/scala/model/ProjectorTest.scala +++ b/image-loader/test/scala/model/ProjectorTest.scala @@ -7,6 +7,7 @@ import com.amazonaws.services.s3.AmazonS3 import com.amazonaws.services.s3.model.ObjectMetadata import com.gu.mediaservice.GridClient import com.gu.mediaservice.lib.auth.Authentication +import com.gu.mediaservice.lib.aws.S3Ops import com.gu.mediaservice.lib.cleanup.ImageProcessor import com.gu.mediaservice.lib.imaging.ImageOperations import com.gu.mediaservice.lib.logging.{LogMarker, MarkerMap} @@ -41,7 +42,7 @@ class ProjectorTest extends AnyFreeSpec with Matchers with ScalaFutures with Moc private val imageOperations = new ImageOperations(ctxPath) - private val config = ImageUploadOpsCfg(new File("/tmp"), 256, 85d, Nil, "img-bucket", "thumb-bucket") + private val config = ImageUploadOpsCfg(new File("/tmp"), 256, 85d, Nil, "img-bucket", S3Ops.s3Endpoint, "thumb-bucket", S3Ops.s3Endpoint) private val maybeEmbedder = None diff --git a/media-api/app/lib/ImageResponse.scala b/media-api/app/lib/ImageResponse.scala index a8cd9df3bd..73824d0169 100644 --- a/media-api/app/lib/ImageResponse.scala +++ b/media-api/app/lib/ImageResponse.scala @@ -83,7 +83,7 @@ class ImageResponse(config: MediaApiConfig, s3Client: S3, usageQuota: UsageQuota val pngUrl: Option[String] = pngFileUri .map(s3Client.signUrl(config.imageBucket, _, image, imageType = OptimisedPng)) - def s3SignedThumbUrl = s3Client.signUrl(config.thumbBucket, fileUri, image, imageType = Thumbnail) + def s3SignedThumbUrl = s3Client.signUrl(config.thumbnailBucket, fileUri, image, imageType = Thumbnail) val thumbUrl = config.cloudFrontDomainThumbBucket .map(domain => s"https://$domain${fileUri.getPath}") diff --git a/media-api/app/lib/MediaApiConfig.scala b/media-api/app/lib/MediaApiConfig.scala index 6258b1faf8..db9f1d9d6a 100644 --- a/media-api/app/lib/MediaApiConfig.scala +++ b/media-api/app/lib/MediaApiConfig.scala @@ -15,7 +15,8 @@ import scala.util.Try case class StoreConfig( storeBucket: String, - storeKey: String + storeKey: String, + storeBucketS3Endpoint: String ) class MediaApiConfig(resources: GridConfigResources) extends CommonConfigWithElastic(resources) { @@ -23,14 +24,11 @@ class MediaApiConfig(resources: GridConfigResources) extends CommonConfigWithEla val usageMailBucket: String = string("s3.usagemail.bucket") val quotaStoreKey: String = string("quota.store.key") - val quotaStoreConfig: StoreConfig = StoreConfig(configBucket, quotaStoreKey) + val quotaStoreConfig: StoreConfig = StoreConfig(configBucket, quotaStoreKey, "s3.amazonaws.com") //Lazy allows this to be empty and not break things unless used somewhere lazy val imgPublishingBucket = string("publishing.image.bucket") - val imageBucket: String = string("s3.image.bucket") - val thumbBucket: String = string("s3.thumb.bucket") - val cloudFrontDomainThumbBucket: Option[String] = stringOpt("cloudfront.domain.thumbbucket") val cloudFrontPrivateKeyBucket: Option[String] = stringOpt("cloudfront.private-key.bucket") val cloudFrontPrivateKeyBucketKey: Option[String] = stringOpt("cloudfront.private-key.key") diff --git a/media-api/app/lib/QuotaStore.scala b/media-api/app/lib/QuotaStore.scala index c79486fbd1..ce1e3f8f5c 100644 --- a/media-api/app/lib/QuotaStore.scala +++ b/media-api/app/lib/QuotaStore.scala @@ -8,8 +8,9 @@ import scala.concurrent.ExecutionContext class QuotaStore( quotaFile: String, bucket: String, - config: MediaApiConfig - )(implicit ec: ExecutionContext) extends BaseStore[String, SupplierUsageQuota](bucket, config)(ec) { + config: MediaApiConfig, + val s3Endpoint: String, + )(implicit ec: ExecutionContext) extends BaseStore[String, SupplierUsageQuota](bucket, config, s3Endpoint)(ec) { def getQuota: Map[String, SupplierUsageQuota] = store.get() diff --git a/media-api/app/lib/UsageQuota.scala b/media-api/app/lib/UsageQuota.scala index bcd4e93e21..c508085fc4 100644 --- a/media-api/app/lib/UsageQuota.scala +++ b/media-api/app/lib/UsageQuota.scala @@ -16,13 +16,15 @@ class UsageQuota(config: MediaApiConfig, scheduler: Scheduler) { val quotaStore = new QuotaStore( config.quotaStoreConfig.storeKey, config.quotaStoreConfig.storeBucket, - config + config, + config.quotaStoreConfig.storeBucketS3Endpoint ) val usageStore = new UsageStore( config.usageMailBucket, config, - quotaStore + quotaStore, + "s3.amazonaws.com" ) def scheduleUpdates(): Unit = { diff --git a/media-api/app/lib/UsageStore.scala b/media-api/app/lib/UsageStore.scala index 2c19044131..a2f3e8130b 100644 --- a/media-api/app/lib/UsageStore.scala +++ b/media-api/app/lib/UsageStore.scala @@ -59,8 +59,9 @@ object UsageStore extends GridLogging { class UsageStore( bucket: String, config: MediaApiConfig, - quotaStore: QuotaStore -)(implicit val ec: ExecutionContext) extends BaseStore[String, UsageStatus](bucket, config) with GridLogging { + quotaStore: QuotaStore, + val s3Endpoint: String +)(implicit val ec: ExecutionContext) extends BaseStore[String, UsageStatus](bucket, config, s3Endpoint) with GridLogging { import UsageStore._ def getUsageStatusForUsageRights(usageRights: UsageRights): Future[UsageStatus] = { diff --git a/rest-lib/src/main/scala/com/gu/mediaservice/lib/auth/provider/ApiKeyAuthenticationProvider.scala b/rest-lib/src/main/scala/com/gu/mediaservice/lib/auth/provider/ApiKeyAuthenticationProvider.scala index e42c25c718..08a306ad0a 100644 --- a/rest-lib/src/main/scala/com/gu/mediaservice/lib/auth/provider/ApiKeyAuthenticationProvider.scala +++ b/rest-lib/src/main/scala/com/gu/mediaservice/lib/auth/provider/ApiKeyAuthenticationProvider.scala @@ -25,7 +25,7 @@ class ApiKeyAuthenticationProvider(configuration: Configuration, resources: Auth var keyStorePlaceholder: Option[KeyStore] = _ override def initialise(): Unit = { - val store = new KeyStore(configuration.get[String]("authKeyStoreBucket"), resources.commonConfig) + val store = new KeyStore(configuration.get[String]("authKeyStoreBucket"), resources.commonConfig, "s3.amazonaws.com") store.scheduleUpdates(resources.actorSystem.scheduler) keyStorePlaceholder = Some(store) } diff --git a/rest-lib/src/test/scala/com/gu/mediaservice/lib/auth/ApiKeyAuthenticationProviderTest.scala b/rest-lib/src/test/scala/com/gu/mediaservice/lib/auth/ApiKeyAuthenticationProviderTest.scala index 3ba9231943..c5308a584e 100644 --- a/rest-lib/src/test/scala/com/gu/mediaservice/lib/auth/ApiKeyAuthenticationProviderTest.scala +++ b/rest-lib/src/test/scala/com/gu/mediaservice/lib/auth/ApiKeyAuthenticationProviderTest.scala @@ -43,7 +43,7 @@ class ApiKeyAuthenticationProviderTest extends AsyncFreeSpec with Matchers with Future.successful(()) } - override def keyStore: KeyStore = new KeyStore("not-used", resources.commonConfig) { + override def keyStore: KeyStore = new KeyStore("not-used", resources.commonConfig, "s3.amazonaws.com") { override def lookupIdentity(key: String)(implicit instance: Instance): Option[ApiAccessor] = { key match { case "key-chuckle" => Some(ApiAccessor("brothers", Internal)) diff --git a/scripts/src/main/scala/com/gu/mediaservice/lib/JsonValueCodecJsValue.scala b/scripts/src/main/scala/com/gu/mediaservice/lib/JsonValueCodecJsValue.scala index da73d03db6..c6ad7b03f8 100644 --- a/scripts/src/main/scala/com/gu/mediaservice/lib/JsonValueCodecJsValue.scala +++ b/scripts/src/main/scala/com/gu/mediaservice/lib/JsonValueCodecJsValue.scala @@ -98,6 +98,7 @@ object JsonValueCodecJsValue { case JsNull => out.writeNull() case _ => + out.writeNull() } } diff --git a/thrall/app/lib/ThrallConfig.scala b/thrall/app/lib/ThrallConfig.scala index 25743ddf67..7b4d3b03ae 100644 --- a/thrall/app/lib/ThrallConfig.scala +++ b/thrall/app/lib/ThrallConfig.scala @@ -56,10 +56,6 @@ object KinesisReceiverConfig { } class ThrallConfig(resources: GridConfigResources) extends CommonConfigWithElastic(resources) { - val imageBucket: String = string("s3.image.bucket") - - val thumbnailBucket: String = string("s3.thumb.bucket") - val maybeReaperBucket: Option[String] = stringOpt("s3.reaper.bucket") val maybeReaperCountPerRun: Option[Int] = intOpt("reaper.countPerRun") diff --git a/thrall/app/lib/ThrallStore.scala b/thrall/app/lib/ThrallStore.scala index a221c0471c..4deb3e5444 100644 --- a/thrall/app/lib/ThrallStore.scala +++ b/thrall/app/lib/ThrallStore.scala @@ -2,4 +2,4 @@ package lib import com.gu.mediaservice.lib -class ThrallStore(config: ThrallConfig) extends lib.ImageIngestOperations(config.imageBucket, config.thumbnailBucket, config, config.isVersionedS3) +class ThrallStore(config: ThrallConfig) extends lib.ImageIngestOperations(config.imageBucket, config.thumbnailBucket, config, config.isVersionedS3, config.imageBucketS3Endpoint, config.thumbnailBucketS3Endpoint) From 64d1d13414ee4c219f54b671b308e665d2917464 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sat, 23 May 2026 18:47:23 +0100 Subject: [PATCH 04/39] Setting up for GCP and AWS clients by introducing s3 end point along side bucket. Bucket should be (bucket, endPoint, client?)  Conflicts:  common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala  common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala  common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala  image-loader/app/model/Uploader.scala  image-loader/test/scala/model/ImageUploadTest.scala  image-loader/test/scala/model/ProjectorTest.scala --- .../com/gu/mediaservice/lib/BaseStore.scala | 6 +- .../lib/ImageIngestOperations.scala | 30 ++--- .../lib/ImageQuarantineOperations.scala | 6 +- .../gu/mediaservice/lib/ImageStorage.scala | 9 +- .../gu/mediaservice/lib/S3ImageStorage.scala | 18 +-- .../gu/mediaservice/lib/auth/KeyStore.scala | 6 +- .../com/gu/mediaservice/lib/aws/S3.scala | 127 ++++++++++-------- .../gu/mediaservice/lib/aws/S3Bucket.scala | 3 + .../lib/config/CommonConfig.scala | 19 ++- cropper/app/lib/CropStore.scala | 6 +- cropper/app/lib/CropperConfig.scala | 8 +- cropper/app/lib/Crops.scala | 4 +- cropper/test/lib/CropsTest.scala | 3 +- image-loader/app/lib/ImageLoaderConfig.scala | 7 +- image-loader/app/lib/ImageLoaderStore.scala | 8 +- image-loader/app/lib/QuarantineStore.scala | 2 +- image-loader/app/model/Projector.scala | 14 +- image-loader/app/model/Uploader.scala | 9 +- .../test/scala/model/ImageUploadTest.scala | 8 +- .../test/scala/model/ProjectorTest.scala | 3 +- media-api/app/controllers/MediaApi.scala | 2 + media-api/app/lib/MediaApiConfig.scala | 14 +- media-api/app/lib/QuotaStore.scala | 6 +- media-api/app/lib/UsageQuota.scala | 3 +- media-api/app/lib/UsageStore.scala | 6 +- .../ApiKeyAuthenticationProvider.scala | 4 +- .../ApiKeyAuthenticationProviderTest.scala | 3 +- thrall/app/lib/ThrallConfig.scala | 4 +- thrall/app/lib/ThrallStore.scala | 2 +- 29 files changed, 182 insertions(+), 158 deletions(-) create mode 100644 common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/BaseStore.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/BaseStore.scala index 31d3cbb610..5a0f2eb36d 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/BaseStore.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/BaseStore.scala @@ -1,7 +1,7 @@ package com.gu.mediaservice.lib import org.apache.pekko.actor.{Cancellable, Scheduler} -import com.gu.mediaservice.lib.aws.S3 +import com.gu.mediaservice.lib.aws.{S3, S3Bucket} import com.gu.mediaservice.lib.config.CommonConfig import com.gu.mediaservice.lib.logging.GridLogging import org.joda.time.DateTime @@ -14,11 +14,9 @@ import scala.concurrent.duration._ import scala.util.control.NonFatal -abstract class BaseStore[TStoreKey, TStoreVal](bucket: String, config: CommonConfig, s3Endpoint: String)(implicit ec: ExecutionContext) +abstract class BaseStore[TStoreKey, TStoreVal](bucket: S3Bucket, config: CommonConfig)(implicit ec: ExecutionContext) extends GridLogging { - def s3Endpoint: String - val s3 = new S3(config) protected val store: AtomicReference[Map[TStoreKey, TStoreVal]] = new AtomicReference(Map.empty) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala index 895e4161bb..f1e6a0f26a 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala @@ -4,7 +4,7 @@ import com.amazonaws.services.s3.model.MultiObjectDeleteException import java.io.File import com.gu.mediaservice.lib.config.CommonConfig -import com.gu.mediaservice.lib.aws.S3Object +import com.gu.mediaservice.lib.aws.{S3Bucket, S3Object} import com.gu.mediaservice.lib.logging.LogMarker import com.gu.mediaservice.model.{Instance, MimeType, Png} import com.typesafe.scalalogging.StrictLogging @@ -21,7 +21,7 @@ object ImageIngestOperations { private def snippetForId(id: String) = id.take(6).mkString("/") + "/" + id } -class ImageIngestOperations(imageBucket: String, thumbnailBucket: String, config: CommonConfig, isVersionedS3: Boolean = false, imageBucketS3Endpoint: String, thumbnailBucketS3Endpoint: String) +class ImageIngestOperations(imageBucket: S3Bucket, thumbnailBucket: S3Bucket, config: CommonConfig, isVersionedS3: Boolean = false) extends S3ImageStorage(config) with StrictLogging { import ImageIngestOperations.{fileKeyFromId, optimisedPngKeyFromId} @@ -38,7 +38,7 @@ class ImageIngestOperations(imageBucket: String, thumbnailBucket: String, config val instanceSpecificKey = instanceAwareOriginalImageKey(storableImage) logger.info(s"Storing original image to instance specific key:$imageBucket / $instanceSpecificKey") storeImage(imageBucket, instanceSpecificKey, storableImage.file, Some(storableImage.mimeType), - storableImage.meta, overwrite = false, s3Endpoint = imageBucketS3Endpoint) + storableImage.meta, overwrite = false) } private def storeThumbnailImage(storableImage: StorableThumbImage) @@ -46,7 +46,7 @@ class ImageIngestOperations(imageBucket: String, thumbnailBucket: String, config val instanceSpecificKey = instanceAwareThumbnailImageKey(storableImage) logger.info(s"Storing thumbnail to instance specific key: $thumbnailBucket / $instanceSpecificKey") storeImage(thumbnailBucket, instanceSpecificKey, storableImage.file, Some(storableImage.mimeType), - overwrite = true, s3Endpoint = thumbnailBucketS3Endpoint) + overwrite = true) } private def storeOptimisedImage(storableImage: StorableOptimisedImage) @@ -54,10 +54,10 @@ class ImageIngestOperations(imageBucket: String, thumbnailBucket: String, config val instanceSpecificKey = optimisedPngKeyFromId(storableImage.id)(storableImage.instance) logger.info(s"Storing optimised image to instance specific key: $thumbnailBucket / $instanceSpecificKey") storeImage(imageBucket, instanceSpecificKey, storableImage.file, Some(storableImage.mimeType), - overwrite = true, s3Endpoint = imageBucketS3Endpoint) + overwrite = true) } - private def bulkDelete(bucket: String, keys: List[String]): Future[Map[String, Boolean]] = keys match { + private def bulkDelete(bucket: S3Bucket, keys: List[String]): Future[Map[String, Boolean]] = keys match { case Nil => Future.successful(Map.empty) case _ => Future { try { @@ -105,38 +105,38 @@ sealed trait ImageWrapper { val instance: Instance } sealed trait StorableImage extends ImageWrapper { - def toProjectedS3Object(thumbBucket: String, thumbBucketEndpoint: String): S3Object = S3Object( - thumbBucket, + def toProjectedS3Object(thumbBucket: S3Bucket): S3Object = S3Object( + thumbBucket.bucket, ImageIngestOperations.fileKeyFromId(id)(instance), file, Some(mimeType), lastModified = None, meta, - s3Endpoint = thumbBucketEndpoint + s3Endpoint = thumbBucket.endpoint ) } case class StorableThumbImage(id: String, file: File, mimeType: MimeType, meta: Map[String, String] = Map.empty, instance: Instance) extends StorableImage case class StorableOriginalImage(id: String, file: File, mimeType: MimeType, lastModified: DateTime, meta: Map[String, String] = Map.empty, instance: Instance) extends StorableImage { - override def toProjectedS3Object(thumbBucket: String, thumbBucketEndpoint: String): S3Object = S3Object( - thumbBucket, + override def toProjectedS3Object(thumbBucket: S3Bucket): S3Object = S3Object( + thumbBucket.bucket, ImageIngestOperations.fileKeyFromId(id)(instance), file, Some(mimeType), lastModified = Some(lastModified), meta, - s3Endpoint = thumbBucketEndpoint + s3Endpoint = thumbBucket.endpoint ) } case class StorableOptimisedImage(id: String, file: File, mimeType: MimeType, meta: Map[String, String] = Map.empty, instance: Instance) extends StorableImage { - override def toProjectedS3Object(thumbBucket: String, thumbBucketEndpoint: String): S3Object = S3Object( - thumbBucket, + override def toProjectedS3Object(thumbBucket: S3Bucket): S3Object = S3Object( + thumbBucket.bucket, ImageIngestOperations.optimisedPngKeyFromId(id)(instance), file, Some(mimeType), lastModified = None, meta = meta, - s3Endpoint = thumbBucketEndpoint + s3Endpoint = thumbBucket.endpoint ) } diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageQuarantineOperations.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageQuarantineOperations.scala index 51b04e980d..eaf6b82979 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageQuarantineOperations.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageQuarantineOperations.scala @@ -2,18 +2,18 @@ package com.gu.mediaservice.lib import java.io.File import com.gu.mediaservice.lib.config.CommonConfig -import com.gu.mediaservice.lib.aws.S3Object +import com.gu.mediaservice.lib.aws.{S3Bucket, S3Object} import com.gu.mediaservice.lib.logging.LogMarker import com.gu.mediaservice.model.{Instance, MimeType} import scala.concurrent.Future -class ImageQuarantineOperations(quarantineBucket: String, config: CommonConfig, isVersionedS3: Boolean = false, s3Endpoint: String) +class ImageQuarantineOperations(quarantineBucket: S3Bucket, config: CommonConfig, isVersionedS3: Boolean = false) extends S3ImageStorage(config) { def storeQuarantineImage(id: String, file: File, mimeType: Option[MimeType], meta: Map[String, String] = Map.empty) (implicit logMarker: LogMarker, instance: Instance): Future[S3Object] = - storeImage(quarantineBucket, ImageIngestOperations.fileKeyFromId(id), file, mimeType, meta, overwrite = true, s3Endpoint = s3Endpoint) + storeImage(quarantineBucket, ImageIngestOperations.fileKeyFromId(id), file, mimeType, meta, overwrite = true) } diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageStorage.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageStorage.scala index fb41afcdfd..010a3c0744 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageStorage.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageStorage.scala @@ -2,11 +2,10 @@ package com.gu.mediaservice.lib import java.util.concurrent.Executors import java.io.File - import scala.concurrent.{ExecutionContext, Future} import scala.concurrent.duration._ import scala.language.postfixOps -import com.gu.mediaservice.lib.aws.S3Object +import com.gu.mediaservice.lib.aws.{S3Bucket, S3Object} import com.gu.mediaservice.lib.logging.LogMarker import com.gu.mediaservice.model.MimeType @@ -37,9 +36,9 @@ trait ImageStorage { /** Store a copy of the given file and return the URI of that copy. * The file can safely be deleted afterwards. */ - def storeImage(bucket: String, id: String, file: File, mimeType: Option[MimeType], - meta: Map[String, String] = Map.empty, overwrite: Boolean, s3Endpoint: String) + def storeImage(bucket: S3Bucket, id: String, file: File, mimeType: Option[MimeType], + meta: Map[String, String] = Map.empty, overwrite: Boolean) (implicit logMarker: LogMarker): Future[S3Object] - def deleteImage(bucket: String, id: String)(implicit logMarker: LogMarker): Future[Unit] + def deleteImage(bucket: S3Bucket, id: String)(implicit logMarker: LogMarker): Future[Unit] } diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/S3ImageStorage.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/S3ImageStorage.scala index 919e688c1c..54b17dba01 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/S3ImageStorage.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/S3ImageStorage.scala @@ -1,6 +1,6 @@ package com.gu.mediaservice.lib -import com.gu.mediaservice.lib.aws.S3 +import com.gu.mediaservice.lib.aws.{S3, S3Bucket} import com.gu.mediaservice.lib.config.CommonConfig import com.gu.mediaservice.lib.logging.{GridLogging, LogMarker} import com.gu.mediaservice.model.MimeType @@ -14,32 +14,32 @@ import scala.concurrent.Future class S3ImageStorage(config: CommonConfig) extends S3(config) with ImageStorage with GridLogging { private val cacheSetting = Some(cacheForever) - def storeImage(bucket: String, id: String, file: File, mimeType: Option[MimeType], - meta: Map[String, String] = Map.empty, overwrite: Boolean, s3Endpoint: String) + def storeImage(bucket: S3Bucket, id: String, file: File, mimeType: Option[MimeType], + meta: Map[String, String] = Map.empty, overwrite: Boolean) (implicit logMarker: LogMarker) = { logger.info(logMarker, s"bucket: $bucket, id: $id, meta: $meta") val eventualObject = if (overwrite) { - store(bucket, id, file, mimeType, meta, cacheSetting, s3Endpoint) + store(bucket, id, file, mimeType, meta, cacheSetting) } else { - storeIfNotPresent(bucket, id, file, mimeType, meta, cacheSetting, s3Endpoint) + storeIfNotPresent(bucket, id, file, mimeType, meta, cacheSetting) } eventualObject.onComplete(o => logger.info(logMarker, s"storeImage completed $o")) eventualObject } - def deleteImage(bucket: String, id: String)(implicit logMarker: LogMarker) = Future { + def deleteImage(bucket: S3Bucket, id: String)(implicit logMarker: LogMarker) = Future { deleteObject(bucket, id) logger.info(logMarker, s"Deleted image $id from bucket $bucket") } - def deleteVersionedImage(bucket: String, id: String)(implicit logMarker: LogMarker) = Future { + def deleteVersionedImage(bucket: S3Bucket, id: String)(implicit logMarker: LogMarker) = Future { val objectVersion = getObjectMetadata(bucket, id).getVersionId deleteVersion(bucket, id, objectVersion) logger.info(logMarker, s"Deleted image $id from bucket $bucket (version: $objectVersion)") } - def deleteFolder(bucket: String, id: String)(implicit logMarker: LogMarker) = Future { - val files = listObjects(bucket, id).getObjectSummaries.asScala + def deleteFolder(bucket: S3Bucket, id: String)(implicit logMarker: LogMarker) = Future { + val files = listObjects(bucket, id).getObjectSummaries.asScala logger.info(s"Found ${files.size} files to delete in folder $id") files.foreach(file => deleteObject(bucket, file.getKey)) logger.info(logMarker, s"Deleting images in folder $id from bucket $bucket") diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/auth/KeyStore.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/auth/KeyStore.scala index 0944203002..ebc43bf09c 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/auth/KeyStore.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/auth/KeyStore.scala @@ -1,13 +1,14 @@ package com.gu.mediaservice.lib.auth import com.gu.mediaservice.lib.BaseStore +import com.gu.mediaservice.lib.aws.S3Bucket import com.gu.mediaservice.lib.config.CommonConfig import com.gu.mediaservice.model.Instance import scala.concurrent.ExecutionContext -class KeyStore(bucket: String, config: CommonConfig, val s3Endpoint: String)(implicit ec: ExecutionContext) - extends BaseStore[String, ApiAccessor](bucket, config, s3Endpoint)(ec) { +class KeyStore(bucket: S3Bucket, config: CommonConfig)(implicit ec: ExecutionContext) + extends BaseStore[String, ApiAccessor](bucket, config)(ec) { def lookupIdentity(key: String)(implicit instance: Instance): Option[ApiAccessor] = store.get().get(instance.id + "/" + key) @@ -18,5 +19,4 @@ class KeyStore(bucket: String, config: CommonConfig, val s3Endpoint: String)(imp private def fetchAll: Map[String, ApiAccessor] = { s3.listObjectKeys(bucket).flatMap(k => getS3Object(k).map(k -> ApiAccessor(_))).toMap } - } diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala index 1f95271e7c..fddaa62c97 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala @@ -62,13 +62,19 @@ object S3Metadata { case class S3ObjectMetadata(contentType: Option[MimeType], cacheControl: Option[String], lastModified: Option[DateTime]) class S3(config: CommonConfig) extends GridLogging with ContentDisposition with RoundedExpiration { - type Bucket = String type Key = String type UserMetadata = Map[String, String] - private lazy val client: AmazonS3 = S3Ops.buildS3Client(config) + val AmazonAwsS3Endpoint: String = S3.AmazonAwsS3Endpoint - def signUrl(bucket: Bucket, url: URI, image: Image, expiration: DateTime = cachableExpiration(), imageType: ImageFileType = Source): String = { + private lazy val amazonS3: AmazonS3 = S3Ops.buildS3Client(config) + + private def clientFor(bucket: S3Bucket): AmazonS3 = { + logger.info("Client for: " + bucket.endpoint) + amazonS3 + } + + def signUrl(bucket: S3Bucket, url: URI, image: Image, expiration: DateTime = cachableExpiration(), imageType: ImageFileType = Source): String = { // get path and remove leading `/` val key: Key = url.getPath.drop(1) @@ -76,60 +82,60 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with val headers = new ResponseHeaderOverrides().withContentDisposition(contentDisposition) - val request = new GeneratePresignedUrlRequest(bucket, key).withExpiration(expiration.toDate).withResponseHeaders(headers) - client.generatePresignedUrl(request).toExternalForm + val request = new GeneratePresignedUrlRequest(bucket.bucket, key).withExpiration(expiration.toDate).withResponseHeaders(headers) + clientFor(bucket).generatePresignedUrl(request).toExternalForm } - def signUrlTony(bucket: Bucket, url: URI, expiration: DateTime = cachableExpiration()): URL = { + def signUrlTony(bucket: S3Bucket, url: URI, expiration: DateTime = cachableExpiration()): URL = { // get path and remove leading `/` val key: Key = url.getPath.drop(1) - val request = new GeneratePresignedUrlRequest(bucket, key).withExpiration(expiration.toDate) - client.generatePresignedUrl(request) + val request = new GeneratePresignedUrlRequest(bucket.bucket, key).withExpiration(expiration.toDate) + clientFor(bucket).generatePresignedUrl(request) } - def copyObject(sourceBucket: Bucket, destinationBucket: Bucket, key: String): CopyObjectResult = { - client.copyObject(sourceBucket, key, destinationBucket, key) + def copyObject(sourceBucket: S3Bucket, destinationBucket: S3Bucket, key: String): CopyObjectResult = { + clientFor(sourceBucket).copyObject(sourceBucket.bucket, key, destinationBucket.bucket, key) } - def generatePresignedRequest(request: GeneratePresignedUrlRequest): URL = { - client.generatePresignedUrl(request) + def generatePresignedRequest(request: GeneratePresignedUrlRequest, bucket: S3Bucket): URL = { + clientFor(bucket).generatePresignedUrl(request) } - def deleteObject(bucket: Bucket, key: String): Unit = { - client.deleteObject(bucket, key) + def deleteObject(bucket: S3Bucket, key: String): Unit = { + clientFor(bucket).deleteObject(bucket.bucket, key) } - def deleteObjects(bucket: Bucket, keys: Seq[Bucket]): DeleteObjectsResult = { - client.deleteObjects( - new DeleteObjectsRequest(bucket).withKeys(keys: _*) + def deleteObjects(bucket: S3Bucket, keys: Seq[String]): DeleteObjectsResult = { + clientFor(bucket).deleteObjects( + new DeleteObjectsRequest(bucket.bucket).withKeys(keys: _*) ) } - def deleteVersion(bucket: Bucket, id: Bucket, objectVersion: Bucket): Unit = { - client.deleteVersion(bucket, id, objectVersion) + def deleteVersion(bucket: S3Bucket, id: String, objectVersion: String): Unit = { + clientFor(bucket).deleteVersion(bucket.bucket, id, objectVersion) } - def doesObjectExist(bucket: Bucket, key: String) = { - client.doesObjectExist(bucket, key) + def doesObjectExist(bucket: S3Bucket, key: String) = { + clientFor(bucket).doesObjectExist(bucket.bucket, key) } - def getObject(bucket: Bucket, url: URI): model.S3Object = { // TODO why can't this just be by bucket + key to remove end point knowledge + def getObject(bucket: S3Bucket, url: URI): model.S3Object = { // TODO why can't this just be by bucket + key to remove end point knowledge // get path and remove leading `/` val key: Key = url.getPath.drop(1) - client.getObject(new GetObjectRequest(bucket, key)) + clientFor(bucket).getObject(new GetObjectRequest(bucket.bucket, key)) } - def getObject(bucket: Bucket, key: String): model.S3Object = { - client.getObject(new GetObjectRequest(bucket, key)) + def getObject(bucket: S3Bucket, key: String): model.S3Object = { + clientFor(bucket).getObject(new GetObjectRequest(bucket.bucket, key)) } - def getObject(bucket: Bucket, obj: S3ObjectSummary): model.S3Object = { - client.getObject(bucket, obj.getKey) + def getObject(bucket: S3Bucket, obj: S3ObjectSummary): model.S3Object = { + clientFor(bucket).getObject(bucket.bucket, obj.getKey) } - def getObjectAsString(bucket: Bucket, key: String): Option[String] = { - val content = client.getObject(new GetObjectRequest(bucket, key)) + def getObjectAsString(bucket: S3Bucket, key: String): Option[String] = { + val content = clientFor(bucket).getObject(new GetObjectRequest(bucket.bucket, key)) val stream = content.getObjectContent try { Some(IOUtils.toString(stream).trim) @@ -143,27 +149,27 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with } } - def getObjectMetadata(bucket: Bucket, id: Bucket): ObjectMetadata = { - client.getObjectMetadata(bucket, id) + def getObjectMetadata(bucket: S3Bucket, id: String): ObjectMetadata = { + clientFor(bucket).getObjectMetadata(bucket.bucket, id) } - def listObjects(bucket: String): ObjectListing = { - client.listObjects(bucket) + def listObjects(bucket: S3Bucket): ObjectListing = { + clientFor(bucket).listObjects(bucket.bucket) } - def listObjects(bucket: String, prefix: String): ObjectListing = { - client.listObjects(bucket, prefix) + def listObjects(bucket: S3Bucket, prefix: String): ObjectListing = { + clientFor(bucket).listObjects(bucket.bucket, prefix) } - def listObjectKeys(bucket: String): Seq[String] = { - client.listObjects(bucket).getObjectSummaries.asScala.map(_.getKey).toSeq + def listObjectKeys(bucket: S3Bucket): Seq[String] = { + clientFor(bucket).listObjects(bucket.bucket).getObjectSummaries.asScala.map(_.getKey).toSeq } - def putObject(bucket: String, key: String, content: String): Unit = { - client.putObject(bucket, key, content) + def putObject(bucket: S3Bucket, key: String, content: String): Unit = { + clientFor(bucket).putObject(bucket.bucket, key, content) } - def store(bucket: Bucket, id: Key, file: File, mimeType: Option[MimeType], meta: UserMetadata = Map.empty, cacheControl: Option[String] = None, s3Endpoint: String) + def store(bucket: S3Bucket, id: Key, file: File, mimeType: Option[MimeType], meta: UserMetadata = Map.empty, cacheControl: Option[String] = None) (implicit ex: ExecutionContext, logMarker: LogMarker): Future[S3Object] = Future { val metadata = new ObjectMetadata @@ -178,54 +184,55 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with ) val markers = logMarker ++ fileMarkers - val req = new PutObjectRequest(bucket, id, file).withMetadata(metadata) + val req = new PutObjectRequest(bucket.bucket, id, file).withMetadata(metadata) Stopwatch(s"S3 client.putObject ($req)"){ + val client = clientFor(bucket) client.putObject(req) // once we've completed the PUT read back to ensure that we are returning reality - val metadata = client.getObjectMetadata(bucket, id) - S3Object(bucket, id, metadata.getContentLength, S3Metadata(metadata), s3Endpoint) + val metadata = client.getObjectMetadata(bucket.bucket, id) + S3Object(bucket.bucket, id, metadata.getContentLength, S3Metadata(metadata), bucket.endpoint) }(markers) } - def storeIfNotPresent(bucket: Bucket, id: Key, file: File, mimeType: Option[MimeType], meta: UserMetadata = Map.empty, cacheControl: Option[String] = None, s3Endpoint: String) + def storeIfNotPresent(bucket: S3Bucket, id: Key, file: File, mimeType: Option[MimeType], meta: UserMetadata = Map.empty, cacheControl: Option[String] = None) (implicit ex: ExecutionContext, logMarker: LogMarker): Future[S3Object] = { Future{ - Some(client.getObjectMetadata(bucket, id)) + Some(clientFor(bucket).getObjectMetadata(bucket.bucket, id)) }.recover { // translate this exception into the object not existing case as3e:AmazonS3Exception if as3e.getStatusCode == 404 => None }.flatMap { case Some(objectMetadata) => logger.info(logMarker, s"Skipping storing of S3 file $id as key is already present in bucket $bucket") - Future.successful(S3Object(bucket, id, objectMetadata.getContentLength, S3Metadata(objectMetadata), s3Endpoint)) + Future.successful(S3Object(bucket.bucket, id, objectMetadata.getContentLength, S3Metadata(objectMetadata), bucket.endpoint)) case None => - store(bucket, id, file, mimeType, meta, cacheControl, s3Endpoint) + store(bucket, id, file, mimeType, meta, cacheControl) } } - def list(bucket: Bucket, prefixDir: String, s3Endpoint: String) + def list(bucket: S3Bucket, prefixDir: String) (implicit ex: ExecutionContext): Future[List[S3Object]] = Future { - val req = new ListObjectsRequest().withBucketName(bucket).withPrefix(s"$prefixDir/") - val listing = client.listObjects(req) + val req = new ListObjectsRequest().withBucketName(bucket.bucket).withPrefix(s"$prefixDir/") + val listing = clientFor(bucket).listObjects(req) val summaries = listing.getObjectSummaries.asScala summaries.map(summary => (summary.getKey, summary)).foldLeft(List[S3Object]()) { case (memo: List[S3Object], (key: String, summary: S3ObjectSummary)) => - S3Object(bucket, key, summary.getSize, getMetadata(bucket, key), s3Endpoint) :: memo + S3Object(bucket.bucket, key, summary.getSize, getMetadata(bucket, key), bucket.endpoint) :: memo } } - def getMetadata(bucket: Bucket, key: Key): S3Metadata = { - val meta = client.getObjectMetadata(bucket, key) + def getMetadata(bucket: S3Bucket, key: Key): S3Metadata = { + val meta = clientFor(bucket).getObjectMetadata(bucket.bucket, key) S3Metadata(meta) } - def getUserMetadata(bucket: Bucket, key: Key): Map[Bucket, Bucket] = - client.getObjectMetadata(bucket, key).getUserMetadata.asScala.toMap + def getUserMetadata(bucket: S3Bucket, key: Key): Map[String, String] = + clientFor(bucket).getObjectMetadata(bucket.bucket, key).getUserMetadata.asScala.toMap - def syncFindKey(bucket: Bucket, prefixName: String): Option[Key] = { - val req = new ListObjectsRequest().withBucketName(bucket).withPrefix(s"$prefixName-") - val listing = client.listObjects(req) + def syncFindKey(bucket: S3Bucket, prefixName: String): Option[Key] = { + val req = new ListObjectsRequest().withBucketName(bucket.bucket).withPrefix(s"$prefixName-") + val listing = clientFor(bucket).listObjects(req) val summaries = listing.getObjectSummaries.asScala summaries.headOption.map(_.getKey) } @@ -250,3 +257,7 @@ object S3Ops { config.withAWSCredentials(builder, localstackAware, maybeRegionOverride).build() } } + +object S3 { + val AmazonAwsS3Endpoint: String = "s3.amazonaws.com" +} diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala new file mode 100644 index 0000000000..2fc155c89c --- /dev/null +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala @@ -0,0 +1,3 @@ +package com.gu.mediaservice.lib.aws + +case class S3Bucket(bucket: String, endpoint: String) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala index 1220268f8c..9d20460982 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala @@ -1,11 +1,10 @@ package com.gu.mediaservice.lib.config -import com.gu.mediaservice.lib.aws.{AwsClientV1BuilderUtils, AwsClientV2BuilderUtils, KinesisSenderConfig, S3Ops} +import com.gu.mediaservice.lib.aws.{AwsClientV1BuilderUtils, AwsClientV2BuilderUtils, KinesisSenderConfig, S3, S3Ops, S3Bucket} import com.gu.mediaservice.model.UsageRightsSpec import com.typesafe.config.Config import com.typesafe.scalalogging.StrictLogging import play.api.{ConfigLoader, Configuration} -import scalaz.NonEmptyList import java.net.URI import java.util.UUID @@ -54,13 +53,12 @@ abstract class CommonConfig(resources: GridConfigResources) extends AwsClientV1B lazy val softDeletedMetadataTable: String = string("dynamo.table.softDelete.metadata") val maybeIngestSqsQueueUrl: Option[String] = stringOpt("sqs.ingest.queue.url") - val maybeIngestBucket: Option[String] = stringOpt("s3.ingest.bucket") - val maybeIngestBucketEndpoint: Option[String] = stringOpt("s3.ingest.bucket").map(_ => S3Ops.s3Endpoint) - val maybeFailBucket: Option[String] = stringOpt("s3.fail.bucket") + val maybeIngestBucket: Option[S3Bucket] = stringOpt("s3.ingest.bucket").map(S3Bucket(_, S3.AmazonAwsS3Endpoint)) + val maybeFailBucket: Option[S3Bucket] = stringOpt("s3.fail.bucket").map(S3Bucket(_, S3.AmazonAwsS3Endpoint)) - val maybeQuarantineBucket: Option[String] = stringOpt("s3.quarantine.bucket") + val maybeQuarantineBucket: Option[S3Bucket] = stringOpt("s3.quarantine.bucket").map(S3Bucket(_, S3.AmazonAwsS3Endpoint)) - val maybeBucketForUIUploads: Option[String] = maybeQuarantineBucket orElse maybeIngestBucket + val maybeBucketForUIUploads: Option[S3Bucket] = maybeQuarantineBucket orElse maybeIngestBucket val maybeUploadLimitInBytes: Option[Int] = intOpt("upload.limit.mb").map(_ * 1024 * 1024) @@ -75,10 +73,9 @@ abstract class CommonConfig(resources: GridConfigResources) extends AwsClientV1B val services = new SingleHostServices(domainRoot) - val imageBucket: String = string("s3.image.bucket") - val imageBucketS3Endpoint: String = "s3.amazonaws.com" - val thumbnailBucket: String = string("s3.thumb.bucket") - val thumbnailBucketS3Endpoint: String = "s3.amazonaws.com" + val imageBucket: S3Bucket = S3Bucket(string("s3.image.bucket"), S3.AmazonAwsS3Endpoint) + val thumbnailBucket: S3Bucket = S3Bucket(string("s3.thumb.bucket"), S3.AmazonAwsS3Endpoint) + val thumbnailBucketS3Endpoint: String = S3.AmazonAwsS3Endpoint /** * Load in a list of domain metadata specifications from configuration. For example: diff --git a/cropper/app/lib/CropStore.scala b/cropper/app/lib/CropStore.scala index cf6eb37e6c..60b3a2672e 100644 --- a/cropper/app/lib/CropStore.scala +++ b/cropper/app/lib/CropStore.scala @@ -15,7 +15,7 @@ class CropStore(config: CropperConfig) extends S3ImageStorage(config) with CropS def storeCropSizing(file: File, filename: String, mimeType: MimeType, crop: Crop, dimensions: Dimensions)(implicit logMarker: LogMarker): Future[Asset] = { val metadata = metadataForCrop(crop, dimensions) - storeImage(config.imgPublishingBucket, filename, file, Some(mimeType), metadata, overwrite = true, config.imgPublishingBucketS3Endpoint) map { s3Object => + storeImage(config.imgPublishingBucket, filename, file, Some(mimeType), metadata, overwrite = true) map { s3Object => Asset( translateImgHost(s3Object.uri), Some(s3Object.size), @@ -27,7 +27,7 @@ class CropStore(config: CropperConfig) extends S3ImageStorage(config) with CropS } def listCrops(id: String, instance: Instance): Future[List[Crop]] = { - list(config.imgPublishingBucket, folderForImagesCrops(id, instance), config.imgPublishingBucketS3Endpoint).map { crops => // TODO crops layout want to be pull up + list(config.imgPublishingBucket, folderForImagesCrops(id, instance)).map { crops => // TODO crops layout want to be pull up crops.foldLeft(Map[String, Crop]()) { case (map, (s3Object)) => { val filename::containingFolder::_ = s3Object.uri.getPath.split("/").reverse.toList @@ -78,7 +78,7 @@ class CropStore(config: CropperConfig) extends S3ImageStorage(config) with CropS def translateImgHost(uri: URI): URI = new URI("https", config.imgPublishingHost, uri.getPath, uri.getFragment) - private def folderForImagesCrops(id: Bucket, instance: Instance) = { + private def folderForImagesCrops(id: String, instance: Instance) = { instance.id + "/" + id } diff --git a/cropper/app/lib/CropperConfig.scala b/cropper/app/lib/CropperConfig.scala index 8aebea551e..395122932c 100644 --- a/cropper/app/lib/CropperConfig.scala +++ b/cropper/app/lib/CropperConfig.scala @@ -1,5 +1,6 @@ package lib +import com.gu.mediaservice.lib.aws.{S3, S3Bucket} import com.gu.mediaservice.lib.config.{CommonConfig, GridConfigResources} import com.gu.mediaservice.model.Instance @@ -7,9 +8,10 @@ import java.io.File class CropperConfig(resources: GridConfigResources) extends CommonConfig(resources) { - val imgPublishingBucket = string("publishing.image.bucket") - val imgPublishingBucketS3Endpoint: String = "s3.amazonaws.com" - + val imgPublishingBucket: S3Bucket = S3Bucket( + string("publishing.image.bucket"), + S3.AmazonAwsS3Endpoint + ) val canDownloadCrop: Boolean = boolean("canDownloadCrop") val imgPublishingHost = string("publishing.image.host") diff --git a/cropper/app/lib/Crops.scala b/cropper/app/lib/Crops.scala index 6c2be74d21..463fa8e1df 100644 --- a/cropper/app/lib/Crops.scala +++ b/cropper/app/lib/Crops.scala @@ -3,7 +3,7 @@ package lib import java.io.File import com.gu.mediaservice.lib.metadata.FileMetadataHelper import com.gu.mediaservice.lib.Files -import com.gu.mediaservice.lib.aws.S3 +import com.gu.mediaservice.lib.aws.{S3, S3Bucket} import com.gu.mediaservice.lib.imaging.{ExportResult, ImageOperations} import com.gu.mediaservice.lib.logging.{GridLogging, LogMarker, Stopwatch} import com.gu.mediaservice.model._ @@ -17,7 +17,7 @@ case object InvalidCropRequest extends Exception("Crop request invalid for image case class MasterCrop(sizing: Future[Asset], file: File, dimensions: Dimensions, aspectRatio: Float) -class Crops(config: CropperConfig, store: CropStore, imageOperations: ImageOperations, imageBucket: String)(implicit ec: ExecutionContext) extends GridLogging { +class Crops(config: CropperConfig, store: CropStore, imageOperations: ImageOperations, imageBucket: S3Bucket)(implicit ec: ExecutionContext) extends GridLogging { import Files._ private val cropQuality = 75d diff --git a/cropper/test/lib/CropsTest.scala b/cropper/test/lib/CropsTest.scala index e348b9a4a6..02752267c0 100644 --- a/cropper/test/lib/CropsTest.scala +++ b/cropper/test/lib/CropsTest.scala @@ -1,5 +1,6 @@ package lib +import com.gu.mediaservice.lib.aws.S3Bucket import com.gu.mediaservice.lib.imaging.ImageOperations import com.gu.mediaservice.model._ import org.scalatest.funspec.AnyFunSpec @@ -50,7 +51,7 @@ class CropsTest extends AnyFunSpec with Matchers with MockitoSugar { private val source: SourceImage = SourceImage("test", mock[Asset], valid = true, mock[ImageMetadata], mock[FileMetadata]) private val bounds: Bounds = Bounds(10, 20, 30, 40) private val outputWidth = 1234 - private val imageBucket = "crops-bucket" + private val imageBucket = S3Bucket("crops-bucket", endpoint = "some-providers-s3-endpoint") it("should should construct a correct address for a master jpg") { val outputFilename = new Crops(config, store, imageOperations, imageBucket) diff --git a/image-loader/app/lib/ImageLoaderConfig.scala b/image-loader/app/lib/ImageLoaderConfig.scala index 7a17360631..ceb923387f 100644 --- a/image-loader/app/lib/ImageLoaderConfig.scala +++ b/image-loader/app/lib/ImageLoaderConfig.scala @@ -1,5 +1,7 @@ package lib +import com.gu.mediaservice.lib.aws.{S3, S3Bucket} + import java.io.File import com.gu.mediaservice.lib.cleanup.{ComposedImageProcessor, ImageProcessor, ImageProcessorResources} import com.gu.mediaservice.lib.config.{CommonConfig, GridConfigResources, ImageProcessorLoader} @@ -13,8 +15,9 @@ class ImageLoaderConfig(resources: GridConfigResources) extends CommonConfig(res val maybeImageReplicaBucket: Option[String] = stringOpt("s3.image.replicaBucket") - val quarantineBucket: Option[String] = stringOpt("s3.quarantine.bucket") - val quarantineBucketS3Endpoint: String = "s3.amazonaws.com" + val quarantineBucket: Option[S3Bucket] = stringOpt("s3.quarantine.bucket").map { bucket => + S3Bucket(bucket, S3.AmazonAwsS3Endpoint) + } val uploadToQuarantineEnabled: Boolean = boolean("upload.quarantine.enabled") val lowerEnvironmentSamplingPercentageAsDecimal = intOpt("s3.sampling.percentage").getOrElse(1) / 100.0 diff --git a/image-loader/app/lib/ImageLoaderStore.scala b/image-loader/app/lib/ImageLoaderStore.scala index fe5f0a0286..1ef435e765 100644 --- a/image-loader/app/lib/ImageLoaderStore.scala +++ b/image-loader/app/lib/ImageLoaderStore.scala @@ -15,7 +15,9 @@ import scala.concurrent.Future class S3FileDoesNotExistException extends Exception() -class ImageLoaderStore(config: ImageLoaderConfig) extends lib.ImageIngestOperations(config.imageBucket, config.thumbnailBucket, config, imageBucketS3Endpoint = config.imageBucketS3Endpoint, thumbnailBucketS3Endpoint = config.thumbnailBucketS3Endpoint) with GridLogging { +class ImageLoaderStore(config: ImageLoaderConfig) extends lib.ImageIngestOperations(config.imageBucket, config.thumbnailBucket, config) with GridLogging { + + private val imageIngestS3Endpoint = AmazonAwsS3Endpoint private def handleNotFound[T](key: String)(doWork: => T)(loggingIfNotFound: => Unit): T = { try { @@ -49,7 +51,7 @@ class ImageLoaderStore(config: ImageLoaderConfig) extends lib.ImageIngestOperati def generatePreSignedUploadUrl(filename: String, expiration: ZonedDateTime, uploadedBy: String, mediaId: String)(implicit instance: Instance): String = { val request = new GeneratePresignedUrlRequest( - config.maybeBucketForUIUploads.get, // bucket + config.maybeBucketForUIUploads.get.bucket, // bucket s"${instance.id}/$uploadedBy/$filename", // key ) .withMethod(HttpMethod.PUT) @@ -58,7 +60,7 @@ class ImageLoaderStore(config: ImageLoaderConfig) extends lib.ImageIngestOperati // sent by the client in manager.js request.putCustomRequestHeader("x-amz-meta-media-id", mediaId) - generatePresignedRequest(request).toString + generatePresignedRequest(request, config.maybeIngestBucket.get).toString } def moveObjectToFailedBucket(key: String)(implicit logMarker: LogMarker) = handleNotFound(key){ diff --git a/image-loader/app/lib/QuarantineStore.scala b/image-loader/app/lib/QuarantineStore.scala index e73e28ead2..0c7e5f515f 100644 --- a/image-loader/app/lib/QuarantineStore.scala +++ b/image-loader/app/lib/QuarantineStore.scala @@ -3,4 +3,4 @@ package lib.storage import lib.ImageLoaderConfig import com.gu.mediaservice.lib -class QuarantineStore(config: ImageLoaderConfig) extends lib.ImageQuarantineOperations(config.quarantineBucket.get, config, s3Endpoint = config.quarantineBucketS3Endpoint) +class QuarantineStore(config: ImageLoaderConfig) extends lib.ImageQuarantineOperations(config.quarantineBucket.get, config) diff --git a/image-loader/app/model/Projector.scala b/image-loader/app/model/Projector.scala index 91ba54362a..2cd8bebd98 100644 --- a/image-loader/app/model/Projector.scala +++ b/image-loader/app/model/Projector.scala @@ -86,7 +86,7 @@ object S3FileExtractedMetadata { } class Projector(config: ImageUploadOpsCfg, - s3: AmazonS3, + s3: AmazonS3, // TODO Not GCP aware! imageOps: ImageOperations, processor: ImageProcessor, auth: Authentication, @@ -100,11 +100,11 @@ class Projector(config: ImageUploadOpsCfg, import ImageIngestOperations.fileKeyFromId val s3Key = fileKeyFromId(imageId) - if (!s3.doesObjectExist(config.originalFileBucket, s3Key)) - throw new NoSuchImageExistsInS3(config.originalFileBucket, s3Key) + if (!s3.doesObjectExist(config.originalFileBucket.bucket, s3Key)) + throw new NoSuchImageExistsInS3(config.originalFileBucket.bucket, s3Key) val s3Source = Stopwatch(s"object exists, getting s3 object at s3://${config.originalFileBucket}/$s3Key to perform Image projection"){ - s3.getObject(config.originalFileBucket, s3Key) + s3.getObject(config.originalFileBucket.bucket, s3Key) }(logMarker) try { @@ -199,7 +199,7 @@ class ImageUploadProjectionOps(config: ImageUploadOpsCfg, private def fetchThumbFile( imageId: String, outFile: File, instance: Instance)(implicit ec: ExecutionContext, logMarker: LogMarker): Future[Option[(File, MimeType)]] = { val key = fileKeyFromId(imageId)(instance) - fetchFile(config.thumbBucket, key, outFile) + fetchFile(config.thumbBucket.bucket, key, outFile) } private def fetchOptimisedFile( @@ -207,10 +207,10 @@ class ImageUploadProjectionOps(config: ImageUploadOpsCfg, )(implicit ec: ExecutionContext, logMarker: LogMarker): Future[Option[(File, MimeType)]] = { val key = optimisedPngKeyFromId(imageId)(instance) - fetchFile(config.originalFileBucket, key, outFile) + fetchFile(config.originalFileBucket.bucket, key, outFile) } - private def fetchFile( + private def fetchFile( // TODO use S3 trait for GCP support! bucket: String, key: String, outFile: File )(implicit ec: ExecutionContext, logMarker: LogMarker): Future[Option[(File, MimeType)]] = { logger.info(logMarker, s"Trying fetch existing image from S3 bucket - $bucket at key $key") diff --git a/image-loader/app/model/Uploader.scala b/image-loader/app/model/Uploader.scala index dd347380d5..cc2845d9b1 100644 --- a/image-loader/app/model/Uploader.scala +++ b/image-loader/app/model/Uploader.scala @@ -8,6 +8,8 @@ import com.gu.mediaservice.lib._ import com.gu.mediaservice.lib.argo.ArgoHelpers import com.gu.mediaservice.lib.auth.Authentication import com.gu.mediaservice.lib.aws.{Embedder, EmbedderMessage, S3Object, UpdateMessage} +import com.gu.mediaservice.lib.{BrowserViewableImage, ImageStorageProps, StorableOptimisedImage, StorableOriginalImage, StorableThumbImage} +import com.gu.mediaservice.lib.aws.{Embedder, S3Bucket, S3Object, S3Vectors, UpdateMessage} import com.gu.mediaservice.lib.cleanup.ImageProcessor import com.gu.mediaservice.lib.formatting._ import com.gu.mediaservice.lib.imaging.ImageOperations @@ -66,10 +68,8 @@ case class ImageUploadOpsCfg( thumbWidth: Int, thumbQuality: Double, transcodedMimeTypes: List[MimeType], - originalFileBucket: String, - originalFileBucketEndpoint: String, - thumbBucket: String, - thumbBucketEndpoint: String + originalFileBucket: S3Bucket, + thumbBucket: S3Bucket, ) case class ImageUploadOpsDependencies( @@ -98,7 +98,6 @@ object Uploader extends GridLogging { config.imageBucket, config.imageBucketS3Endpoint, config.thumbnailBucket, - config.thumbnailBucketS3Endpoint ) } diff --git a/image-loader/test/scala/model/ImageUploadTest.scala b/image-loader/test/scala/model/ImageUploadTest.scala index cea1d8cedb..6dc4674240 100644 --- a/image-loader/test/scala/model/ImageUploadTest.scala +++ b/image-loader/test/scala/model/ImageUploadTest.scala @@ -1,6 +1,10 @@ package model import com.drew.imaging.ImageProcessingException +import com.gu.mediaservice.lib.aws.{S3Metadata, S3Object, S3ObjectMetadata} +import com.gu.mediaservice.lib.{StorableImage, StorableOptimisedImage, StorableOriginalImage, StorableThumbImage} +import com.gu.mediaservice.lib.aws.{EmbedderMessage, S3Metadata, S3Object, S3ObjectMetadata, S3Ops} +import com.gu.mediaservice.lib.aws.{S3, S3Bucket, S3Metadata, S3Object, S3ObjectMetadata, S3Ops} import com.gu.mediaservice.lib.aws.{S3Metadata, S3Object, S3ObjectMetadata, S3Ops} import com.gu.mediaservice.lib.cleanup.ImageProcessor import com.gu.mediaservice.lib.imaging.ImageOperations @@ -32,7 +36,7 @@ class ImageUploadTest extends AsyncFunSuite with Matchers with MockitoSugar { private implicit val logMarker: MockLogMarker = new MockLogMarker() // For mime type info, see https://github.com/guardian/grid/pull/2568 val tempDir = new File("/tmp") - val mockConfig: ImageUploadOpsCfg = ImageUploadOpsCfg(tempDir, 256, 85d, List(Tiff), "img-bucket", S3Ops.s3Endpoint, "thumb-bucket", S3Ops.s3Endpoint) + val mockConfig: ImageUploadOpsCfg = ImageUploadOpsCfg(tempDir, 256, 85d, List(Tiff), S3Bucket("img-bucket", S3.AmazonAwsS3Endpoint), S3Bucket("thumb-bucket", S3.AmazonAwsS3Endpoint)) /** * @todo: I flailed about until I found a path that worked, but @@ -53,7 +57,7 @@ class ImageUploadTest extends AsyncFunSuite with Matchers with MockitoSugar { def mockStore = (a: StorableImage) => Future.successful( - S3Object("madeupname", "madeupkey", a.file, Some(a.mimeType), None, a.meta, None, S3Ops.s3Endpoint) + S3Object("madeupname", "madeupkey", a.file, Some(a.mimeType), None, a.meta, None, S3.AmazonAwsS3Endpoint) ) def storeOrProjectOriginalFile: StorableOriginalImage => Future[S3Object] = mockStore diff --git a/image-loader/test/scala/model/ProjectorTest.scala b/image-loader/test/scala/model/ProjectorTest.scala index 0d8f565ff4..03591284f6 100644 --- a/image-loader/test/scala/model/ProjectorTest.scala +++ b/image-loader/test/scala/model/ProjectorTest.scala @@ -7,6 +7,7 @@ import com.amazonaws.services.s3.AmazonS3 import com.amazonaws.services.s3.model.ObjectMetadata import com.gu.mediaservice.GridClient import com.gu.mediaservice.lib.auth.Authentication +import com.gu.mediaservice.lib.aws.{Embedder, S3, S3Bucket, S3Vectors} import com.gu.mediaservice.lib.aws.S3Ops import com.gu.mediaservice.lib.cleanup.ImageProcessor import com.gu.mediaservice.lib.imaging.ImageOperations @@ -42,7 +43,7 @@ class ProjectorTest extends AnyFreeSpec with Matchers with ScalaFutures with Moc private val imageOperations = new ImageOperations(ctxPath) - private val config = ImageUploadOpsCfg(new File("/tmp"), 256, 85d, Nil, "img-bucket", S3Ops.s3Endpoint, "thumb-bucket", S3Ops.s3Endpoint) + private val config = ImageUploadOpsCfg(new File("/tmp"), 256, 85d, Nil, S3Bucket("img-bucket", S3.AmazonAwsS3Endpoint), S3Bucket("thumb-bucket", S3.AmazonAwsS3Endpoint)) private val maybeEmbedder = None diff --git a/media-api/app/controllers/MediaApi.scala b/media-api/app/controllers/MediaApi.scala index 3f268a1dd8..4d9bfbd573 100644 --- a/media-api/app/controllers/MediaApi.scala +++ b/media-api/app/controllers/MediaApi.scala @@ -9,6 +9,8 @@ import com.gu.mediaservice.lib.auth.Permissions.{ArchiveImages, DeleteCropsOrUsa import com.gu.mediaservice.lib.auth._ import com.gu.mediaservice.lib.auth.provider.ApiKeyAuthenticationProvider import com.gu.mediaservice.lib.aws._ +import com.gu.mediaservice.lib.aws.{ContentDisposition, Embedder, ThrallMessageSender, UpdateMessage} +import com.gu.mediaservice.lib.aws.{ContentDisposition, S3, ThrallMessageSender, UpdateMessage} import com.gu.mediaservice.lib.config.InstanceForRequest import com.gu.mediaservice.lib.events.UsageEvents import com.gu.mediaservice.lib.formatting.printDateTime diff --git a/media-api/app/lib/MediaApiConfig.scala b/media-api/app/lib/MediaApiConfig.scala index db9f1d9d6a..44a911cd31 100644 --- a/media-api/app/lib/MediaApiConfig.scala +++ b/media-api/app/lib/MediaApiConfig.scala @@ -1,5 +1,6 @@ package lib +import com.gu.mediaservice.lib.aws.{S3, S3Bucket} import com.gu.mediaservice.lib.config.{CommonConfigWithElastic, GridConfigResources} import com.gu.mediaservice.lib.elasticsearch.filters import com.sksamuel.elastic4s.ElasticApi.{matchPhraseQuery, should} @@ -14,23 +15,22 @@ import scala.collection.immutable import scala.util.Try case class StoreConfig( - storeBucket: String, + storeBucket: S3Bucket, storeKey: String, - storeBucketS3Endpoint: String ) class MediaApiConfig(resources: GridConfigResources) extends CommonConfigWithElastic(resources) { - val configBucket: String = string("s3.config.bucket") - val usageMailBucket: String = string("s3.usagemail.bucket") + val configBucket: S3Bucket = S3Bucket(string("s3.config.bucket"), S3.AmazonAwsS3Endpoint) + val usageMailBucket: S3Bucket = S3Bucket(string("s3.usagemail.bucket"), S3.AmazonAwsS3Endpoint) val quotaStoreKey: String = string("quota.store.key") - val quotaStoreConfig: StoreConfig = StoreConfig(configBucket, quotaStoreKey, "s3.amazonaws.com") + val quotaStoreConfig: StoreConfig = StoreConfig(configBucket, quotaStoreKey) //Lazy allows this to be empty and not break things unless used somewhere - lazy val imgPublishingBucket = string("publishing.image.bucket") + lazy val imgPublishingBucket: S3Bucket = S3Bucket(string("publishing.image.bucket"), S3.AmazonAwsS3Endpoint) val cloudFrontDomainThumbBucket: Option[String] = stringOpt("cloudfront.domain.thumbbucket") - val cloudFrontPrivateKeyBucket: Option[String] = stringOpt("cloudfront.private-key.bucket") + val cloudFrontPrivateKeyBucket: Option[S3Bucket] = stringOpt("cloudfront.private-key.bucket").map(S3Bucket(_, S3.AmazonAwsS3Endpoint)) val cloudFrontPrivateKeyBucketKey: Option[String] = stringOpt("cloudfront.private-key.key") val cloudFrontKeyPairId: Option[String] = stringOpt("cloudfront.keypair.id") diff --git a/media-api/app/lib/QuotaStore.scala b/media-api/app/lib/QuotaStore.scala index ce1e3f8f5c..1606586554 100644 --- a/media-api/app/lib/QuotaStore.scala +++ b/media-api/app/lib/QuotaStore.scala @@ -1,16 +1,16 @@ package lib import com.gu.mediaservice.lib.BaseStore +import com.gu.mediaservice.lib.aws.S3Bucket import play.api.libs.json.Json import scala.concurrent.ExecutionContext class QuotaStore( quotaFile: String, - bucket: String, + bucket: S3Bucket, config: MediaApiConfig, - val s3Endpoint: String, - )(implicit ec: ExecutionContext) extends BaseStore[String, SupplierUsageQuota](bucket, config, s3Endpoint)(ec) { + )(implicit ec: ExecutionContext) extends BaseStore[String, SupplierUsageQuota](bucket, config)(ec) { def getQuota: Map[String, SupplierUsageQuota] = store.get() diff --git a/media-api/app/lib/UsageQuota.scala b/media-api/app/lib/UsageQuota.scala index c508085fc4..0dc049fc77 100644 --- a/media-api/app/lib/UsageQuota.scala +++ b/media-api/app/lib/UsageQuota.scala @@ -2,6 +2,7 @@ package lib import org.apache.pekko.actor.Scheduler import com.gu.mediaservice.lib.FeatureToggle +import com.gu.mediaservice.lib.aws.S3 import com.gu.mediaservice.model.UsageRights import scala.concurrent.Await @@ -17,14 +18,12 @@ class UsageQuota(config: MediaApiConfig, scheduler: Scheduler) { config.quotaStoreConfig.storeKey, config.quotaStoreConfig.storeBucket, config, - config.quotaStoreConfig.storeBucketS3Endpoint ) val usageStore = new UsageStore( config.usageMailBucket, config, quotaStore, - "s3.amazonaws.com" ) def scheduleUpdates(): Unit = { diff --git a/media-api/app/lib/UsageStore.scala b/media-api/app/lib/UsageStore.scala index a2f3e8130b..c8b55defa0 100644 --- a/media-api/app/lib/UsageStore.scala +++ b/media-api/app/lib/UsageStore.scala @@ -1,6 +1,7 @@ package lib import com.gu.mediaservice.lib.BaseStore +import com.gu.mediaservice.lib.aws.S3Bucket import com.gu.mediaservice.lib.logging.GridLogging import com.gu.mediaservice.model.{Agencies, Agency, UsageRights} import org.joda.time.DateTime @@ -57,11 +58,10 @@ object UsageStore extends GridLogging { } class UsageStore( - bucket: String, + bucket: S3Bucket, config: MediaApiConfig, quotaStore: QuotaStore, - val s3Endpoint: String -)(implicit val ec: ExecutionContext) extends BaseStore[String, UsageStatus](bucket, config, s3Endpoint) with GridLogging { +)(implicit val ec: ExecutionContext) extends BaseStore[String, UsageStatus](bucket, config) with GridLogging { import UsageStore._ def getUsageStatusForUsageRights(usageRights: UsageRights): Future[UsageStatus] = { diff --git a/rest-lib/src/main/scala/com/gu/mediaservice/lib/auth/provider/ApiKeyAuthenticationProvider.scala b/rest-lib/src/main/scala/com/gu/mediaservice/lib/auth/provider/ApiKeyAuthenticationProvider.scala index 08a306ad0a..e5ada4c33f 100644 --- a/rest-lib/src/main/scala/com/gu/mediaservice/lib/auth/provider/ApiKeyAuthenticationProvider.scala +++ b/rest-lib/src/main/scala/com/gu/mediaservice/lib/auth/provider/ApiKeyAuthenticationProvider.scala @@ -2,6 +2,7 @@ package com.gu.mediaservice.lib.auth.provider import com.gu.mediaservice.lib.auth.Authentication.{MachinePrincipal, Principal} import com.gu.mediaservice.lib.auth.provider.ApiKeyAuthenticationProvider.{ApiKeyInstance, KindeIdKey} import com.gu.mediaservice.lib.auth.{ApiAccessor, KeyStore} +import com.gu.mediaservice.lib.aws.{S3, S3Bucket} import com.gu.mediaservice.lib.config.InstanceForRequest import com.gu.mediaservice.lib.events.UsageEvents import com.gu.mediaservice.model.Instance @@ -25,7 +26,8 @@ class ApiKeyAuthenticationProvider(configuration: Configuration, resources: Auth var keyStorePlaceholder: Option[KeyStore] = _ override def initialise(): Unit = { - val store = new KeyStore(configuration.get[String]("authKeyStoreBucket"), resources.commonConfig, "s3.amazonaws.com") + val authKeyStoreBucket = S3Bucket(configuration.get[String]("authKeyStoreBucket"), S3.AmazonAwsS3Endpoint) + val store = new KeyStore(authKeyStoreBucket, resources.commonConfig) store.scheduleUpdates(resources.actorSystem.scheduler) keyStorePlaceholder = Some(store) } diff --git a/rest-lib/src/test/scala/com/gu/mediaservice/lib/auth/ApiKeyAuthenticationProviderTest.scala b/rest-lib/src/test/scala/com/gu/mediaservice/lib/auth/ApiKeyAuthenticationProviderTest.scala index c5308a584e..220302deee 100644 --- a/rest-lib/src/test/scala/com/gu/mediaservice/lib/auth/ApiKeyAuthenticationProviderTest.scala +++ b/rest-lib/src/test/scala/com/gu/mediaservice/lib/auth/ApiKeyAuthenticationProviderTest.scala @@ -3,6 +3,7 @@ package com.gu.mediaservice.lib.auth import org.apache.pekko.actor.ActorSystem import com.gu.mediaservice.lib.auth.Authentication.MachinePrincipal import com.gu.mediaservice.lib.auth.provider.{ApiKeyAuthenticationProvider, Authenticated, AuthenticationProviderResources, Invalid, NotAuthenticated, NotAuthorised} +import com.gu.mediaservice.lib.aws.{S3, S3Bucket} import com.gu.mediaservice.lib.config.{CommonConfig, GridConfigResources} import com.gu.mediaservice.lib.events.UsageEvents import com.gu.mediaservice.model.Instance @@ -43,7 +44,7 @@ class ApiKeyAuthenticationProviderTest extends AsyncFreeSpec with Matchers with Future.successful(()) } - override def keyStore: KeyStore = new KeyStore("not-used", resources.commonConfig, "s3.amazonaws.com") { + override def keyStore: KeyStore = new KeyStore(S3Bucket("not-used", S3.AmazonAwsS3Endpoint), resources.commonConfig) { override def lookupIdentity(key: String)(implicit instance: Instance): Option[ApiAccessor] = { key match { case "key-chuckle" => Some(ApiAccessor("brothers", Internal)) diff --git a/thrall/app/lib/ThrallConfig.scala b/thrall/app/lib/ThrallConfig.scala index 7b4d3b03ae..1a08923f9f 100644 --- a/thrall/app/lib/ThrallConfig.scala +++ b/thrall/app/lib/ThrallConfig.scala @@ -1,6 +1,6 @@ package lib -import com.gu.mediaservice.lib.aws.AwsClientV2BuilderUtils +import com.gu.mediaservice.lib.aws.{AwsClientV2BuilderUtils, S3, S3Bucket} import com.gu.mediaservice.lib.cleanup.ReapableEligibiltyResources import com.gu.mediaservice.lib.config.{CommonConfigWithElastic, GridConfigResources, ReapableEligibilityLoader} import com.gu.mediaservice.lib.elasticsearch.ReapableEligibility @@ -56,7 +56,7 @@ object KinesisReceiverConfig { } class ThrallConfig(resources: GridConfigResources) extends CommonConfigWithElastic(resources) { - val maybeReaperBucket: Option[String] = stringOpt("s3.reaper.bucket") + val maybeReaperBucket: Option[S3Bucket] = stringOpt("s3.reaper.bucket").map(S3Bucket(_, S3.AmazonAwsS3Endpoint)) val maybeReaperCountPerRun: Option[Int] = intOpt("reaper.countPerRun") val metadataTopicArn: String = string("indexed.image.sns.topic.arn") diff --git a/thrall/app/lib/ThrallStore.scala b/thrall/app/lib/ThrallStore.scala index 4deb3e5444..a221c0471c 100644 --- a/thrall/app/lib/ThrallStore.scala +++ b/thrall/app/lib/ThrallStore.scala @@ -2,4 +2,4 @@ package lib import com.gu.mediaservice.lib -class ThrallStore(config: ThrallConfig) extends lib.ImageIngestOperations(config.imageBucket, config.thumbnailBucket, config, config.isVersionedS3, config.imageBucketS3Endpoint, config.thumbnailBucketS3Endpoint) +class ThrallStore(config: ThrallConfig) extends lib.ImageIngestOperations(config.imageBucket, config.thumbnailBucket, config, config.isVersionedS3) From 29aa7fab206db1025e8f75554ba52de3b1a903cc Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Tue, 24 Feb 2026 22:10:36 +0000 Subject: [PATCH 05/39] Image projector is bucket S3 endpoint aware. --- image-loader/app/ImageLoaderComponents.scala | 4 +- image-loader/app/lib/ImageLoaderStore.scala | 3 +- image-loader/app/model/Projector.scala | 50 +++++++++---------- image-loader/app/model/Uploader.scala | 1 - .../test/scala/model/ProjectorTest.scala | 3 +- 5 files changed, 28 insertions(+), 33 deletions(-) diff --git a/image-loader/app/ImageLoaderComponents.scala b/image-loader/app/ImageLoaderComponents.scala index 24bc85d0e7..6f8b255498 100644 --- a/image-loader/app/ImageLoaderComponents.scala +++ b/image-loader/app/ImageLoaderComponents.scala @@ -1,5 +1,6 @@ import com.gu.mediaservice.GridClient import com.gu.mediaservice.lib.aws.{Bedrock, Embedder, S3Vectors, SimpleSqsMessageConsumer} +import com.gu.mediaservice.lib.aws.{Bedrock, S3, S3Vectors, SimpleSqsMessageConsumer, Embedder} import com.gu.mediaservice.lib.imaging.ImageOperations import com.gu.mediaservice.lib.logging.GridLogging import com.gu.mediaservice.lib.play.GridComponents @@ -35,7 +36,8 @@ class ImageLoaderComponents(context: Context) extends GridComponents(context, ne } val uploader = new Uploader(store, config, imageOperations, notifications, maybeEmbedder, imageProcessor, gridClient, auth) - val projector = Projector(config, imageOperations, imageProcessor, auth, maybeEmbedder) + val s3 = new S3(config) + val projector = Projector(config, imageOperations, imageProcessor, auth, maybeEmbedder, s3) val quarantineUploader: Option[QuarantineUploader] = config.maybeQuarantineBucket.map(_ => new QuarantineUploader(new QuarantineStore(config), config) ) diff --git a/image-loader/app/lib/ImageLoaderStore.scala b/image-loader/app/lib/ImageLoaderStore.scala index 1ef435e765..eb66b8d864 100644 --- a/image-loader/app/lib/ImageLoaderStore.scala +++ b/image-loader/app/lib/ImageLoaderStore.scala @@ -44,8 +44,7 @@ class ImageLoaderStore(config: ImageLoaderConfig) extends lib.ImageIngestOperati s"${instance.id}/$uploader/$filename", file, mimeType = None, // we don't care as this is just the queue bucket - meta = s3Meta, - s3Endpoint = config.maybeIngestBucketEndpoint.get + meta = s3Meta ) } diff --git a/image-loader/app/model/Projector.scala b/image-loader/app/model/Projector.scala index 2cd8bebd98..82bdf3ad09 100644 --- a/image-loader/app/model/Projector.scala +++ b/image-loader/app/model/Projector.scala @@ -1,39 +1,35 @@ package model -import java.io.{File, FileOutputStream} -import com.amazonaws.services.s3.AmazonS3 -import com.gu.mediaservice.{GridClient, ImageDataMerger} -import com.gu.mediaservice.lib.auth.Authentication -import com.amazonaws.services.s3.model.{GetObjectRequest, ObjectMetadata, S3Object => AwsS3Object} +import com.amazonaws.services.s3.model.{ObjectMetadata, S3Object => AwsS3Object} import com.gu.mediaservice.lib.ImageIngestOperations.{fileKeyFromId, optimisedPngKeyFromId} -import com.gu.mediaservice.lib.{ImageIngestOperations, ImageStorageProps, StorableOptimisedImage, StorableOriginalImage, StorableThumbImage} -import com.gu.mediaservice.lib.aws.{Embedder, EmbedderMessage, S3Ops} +import com.gu.mediaservice.lib.auth.Authentication +import com.gu.mediaservice.lib.aws.{Embedder, EmbedderMessage, S3, S3Bucket} import com.gu.mediaservice.lib.cleanup.ImageProcessor import com.gu.mediaservice.lib.config.InstanceForRequest import com.gu.mediaservice.lib.imaging.ImageOperations import com.gu.mediaservice.lib.logging.{GridLogging, LogMarker, Stopwatch} import com.gu.mediaservice.lib.net.URI +import com.gu.mediaservice.lib._ import com.gu.mediaservice.model.{Image, Instance, MimeType, UploadInfo} +import com.gu.mediaservice.{GridClient, ImageDataMerger} import lib.imaging.{MimeTypeDetection, NoSuchImageExistsInS3} import lib.{DigestedFile, ImageLoaderConfig} import model.upload.UploadRequest import org.apache.commons.io.IOUtils import org.joda.time.{DateTime, DateTimeZone} -import play.api.libs.ws.WSRequest -import software.amazon.awssdk.services.s3vectors.model.PutVectorsResponse -import play.api.mvc.RequestHeader +import _root_.play.api.libs.ws.WSRequest -import java.nio.file.Path -import scala.jdk.CollectionConverters._ +import java.io.{File, FileOutputStream} import scala.concurrent.duration.Duration import scala.concurrent.{Await, ExecutionContext, Future} +import scala.jdk.CollectionConverters._ object Projector { import Uploader.toImageUploadOpsCfg - def apply(config: ImageLoaderConfig, imageOps: ImageOperations, processor: ImageProcessor, auth: Authentication, maybeEmbedder: Option[Embedder])(implicit ec: ExecutionContext): Projector - = new Projector(toImageUploadOpsCfg(config), S3Ops.buildS3Client(config), imageOps, processor, auth, maybeEmbedder) + def apply(config: ImageLoaderConfig, imageOps: ImageOperations, processor: ImageProcessor, auth: Authentication, maybeEmbedder: Option[Embedder], s3: S3)(implicit ec: ExecutionContext): Projector + = new Projector(toImageUploadOpsCfg(config), s3, imageOps, processor, auth, maybeEmbedder) } case class S3FileExtractedMetadata( @@ -86,7 +82,7 @@ object S3FileExtractedMetadata { } class Projector(config: ImageUploadOpsCfg, - s3: AmazonS3, // TODO Not GCP aware! + s3: S3, imageOps: ImageOperations, processor: ImageProcessor, auth: Authentication, @@ -100,11 +96,11 @@ class Projector(config: ImageUploadOpsCfg, import ImageIngestOperations.fileKeyFromId val s3Key = fileKeyFromId(imageId) - if (!s3.doesObjectExist(config.originalFileBucket.bucket, s3Key)) + if (!s3.doesObjectExist(config.originalFileBucket, s3Key)) throw new NoSuchImageExistsInS3(config.originalFileBucket.bucket, s3Key) val s3Source = Stopwatch(s"object exists, getting s3 object at s3://${config.originalFileBucket}/$s3Key to perform Image projection"){ - s3.getObject(config.originalFileBucket.bucket, s3Key) + s3.getObject(config.originalFileBucket, s3Key) }(logMarker) try { @@ -165,11 +161,11 @@ class Projector(config: ImageUploadOpsCfg, class ImageUploadProjectionOps(config: ImageUploadOpsCfg, imageOps: ImageOperations, processor: ImageProcessor, - s3: AmazonS3, + s3: S3, maybeEmbedder: Option[Embedder], ) extends GridLogging { - import Uploader.{fromUploadRequestShared, toMetaMap} + import Uploader.fromUploadRequestShared def projectImageFromUploadRequest(uploadRequest: UploadRequest) @@ -188,18 +184,18 @@ class ImageUploadProjectionOps(config: ImageUploadOpsCfg, } private def projectOriginalFileAsS3Model(storableOriginalImage: StorableOriginalImage) = - Future.successful(storableOriginalImage.toProjectedS3Object(config.originalFileBucket, config.originalFileBucketEndpoint)) + Future.successful(storableOriginalImage.toProjectedS3Object(config.originalFileBucket)) private def projectThumbnailFileAsS3Model(storableThumbImage: StorableThumbImage) = - Future.successful(storableThumbImage.toProjectedS3Object(config.thumbBucket, config.thumbBucketEndpoint)) + Future.successful(storableThumbImage.toProjectedS3Object(config.thumbBucket)) private def projectOptimisedPNGFileAsS3Model(storableOptimisedImage: StorableOptimisedImage) = - Future.successful(storableOptimisedImage.toProjectedS3Object(config.originalFileBucket, config.originalFileBucketEndpoint)) + Future.successful(storableOptimisedImage.toProjectedS3Object(config.originalFileBucket)) private def fetchThumbFile( imageId: String, outFile: File, instance: Instance)(implicit ec: ExecutionContext, logMarker: LogMarker): Future[Option[(File, MimeType)]] = { val key = fileKeyFromId(imageId)(instance) - fetchFile(config.thumbBucket.bucket, key, outFile) + fetchFile(config.thumbBucket, key, outFile) } private def fetchOptimisedFile( @@ -207,11 +203,11 @@ class ImageUploadProjectionOps(config: ImageUploadOpsCfg, )(implicit ec: ExecutionContext, logMarker: LogMarker): Future[Option[(File, MimeType)]] = { val key = optimisedPngKeyFromId(imageId)(instance) - fetchFile(config.originalFileBucket.bucket, key, outFile) + fetchFile(config.originalFileBucket, key, outFile) } - private def fetchFile( // TODO use S3 trait for GCP support! - bucket: String, key: String, outFile: File + private def fetchFile( + bucket: S3Bucket, key: String, outFile: File )(implicit ec: ExecutionContext, logMarker: LogMarker): Future[Option[(File, MimeType)]] = { logger.info(logMarker, s"Trying fetch existing image from S3 bucket - $bucket at key $key") val doesFileExist = Future { s3.doesObjectExist(bucket, key) } recover { case _ => false } @@ -220,7 +216,7 @@ class ImageUploadProjectionOps(config: ImageUploadOpsCfg, logger.warn(logMarker, s"image did not exist in bucket $bucket at key $key") Future.successful(None) // falls back to creating from original file case true => - val obj = s3.getObject(new GetObjectRequest(bucket, key)) + val obj = s3.getObject(bucket, key) val fos = new FileOutputStream(outFile) try { IOUtils.copy(obj.getObjectContent, fos) diff --git a/image-loader/app/model/Uploader.scala b/image-loader/app/model/Uploader.scala index cc2845d9b1..7b22de8b41 100644 --- a/image-loader/app/model/Uploader.scala +++ b/image-loader/app/model/Uploader.scala @@ -96,7 +96,6 @@ object Uploader extends GridLogging { config.thumbQuality, config.transcodedMimeTypes, config.imageBucket, - config.imageBucketS3Endpoint, config.thumbnailBucket, ) } diff --git a/image-loader/test/scala/model/ProjectorTest.scala b/image-loader/test/scala/model/ProjectorTest.scala index 03591284f6..f145a237be 100644 --- a/image-loader/test/scala/model/ProjectorTest.scala +++ b/image-loader/test/scala/model/ProjectorTest.scala @@ -3,7 +3,6 @@ package model import java.io.File import java.net.URI import java.util.{Date, UUID} -import com.amazonaws.services.s3.AmazonS3 import com.amazonaws.services.s3.model.ObjectMetadata import com.gu.mediaservice.GridClient import com.gu.mediaservice.lib.auth.Authentication @@ -47,7 +46,7 @@ class ProjectorTest extends AnyFreeSpec with Matchers with ScalaFutures with Moc private val maybeEmbedder = None - private val s3 = mock[AmazonS3] + private val s3 = mock[S3] private val auth = mock[Authentication] private val projector = new Projector(config, s3, imageOperations, ImageProcessor.identity, auth, maybeEmbedder) From 27ac5c89819428ce48b72a484726876a498e9ac3 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Fri, 29 Nov 2024 22:17:03 +0000 Subject: [PATCH 06/39] Image bucket takes end point config from config. Update fixture config with new bucket conf. --- .../scala/com/gu/mediaservice/lib/config/CommonConfig.scala | 2 +- common-lib/src/test/resources/application.conf | 4 +++- media-api/test/lib/elasticsearch/Fixtures.scala | 3 ++- rest-lib/src/test/resources/application.conf | 3 +++ 4 files changed, 9 insertions(+), 3 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala index 9d20460982..b95e98a6c1 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala @@ -73,7 +73,7 @@ abstract class CommonConfig(resources: GridConfigResources) extends AwsClientV1B val services = new SingleHostServices(domainRoot) - val imageBucket: S3Bucket = S3Bucket(string("s3.image.bucket"), S3.AmazonAwsS3Endpoint) + val imageBucket: S3Bucket = S3Bucket(string("s3.image.bucket.name") ,stringOpt("s3.image.bucket.endpoint").get) val thumbnailBucket: S3Bucket = S3Bucket(string("s3.thumb.bucket"), S3.AmazonAwsS3Endpoint) val thumbnailBucketS3Endpoint: String = S3.AmazonAwsS3Endpoint diff --git a/common-lib/src/test/resources/application.conf b/common-lib/src/test/resources/application.conf index e3724393da..c178da2149 100644 --- a/common-lib/src/test/resources/application.conf +++ b/common-lib/src/test/resources/application.conf @@ -62,6 +62,8 @@ instance.service.instances="" usageEvents.queue.name="" -s3.image.bucket="images" +s3.image.bucket.name="images" +s3.image.bucket.endpoint="some-providers-s3-endpoint" s3.thumb.bucket="thumbs" + diff --git a/media-api/test/lib/elasticsearch/Fixtures.scala b/media-api/test/lib/elasticsearch/Fixtures.scala index d38c28d497..3364ff9242 100644 --- a/media-api/test/lib/elasticsearch/Fixtures.scala +++ b/media-api/test/lib/elasticsearch/Fixtures.scala @@ -35,7 +35,8 @@ trait Fixtures { "es.index.aliases.current", "es.index.aliases.migration", "es6.url", - "s3.image.bucket", + "s3.image.bucket.name", + "s3.image.bucket.endpoint", "s3.thumb.bucket", "grid.stage", "grid.appName", diff --git a/rest-lib/src/test/resources/application.conf b/rest-lib/src/test/resources/application.conf index 930c6a00d5..cfd0579a07 100644 --- a/rest-lib/src/test/resources/application.conf +++ b/rest-lib/src/test/resources/application.conf @@ -3,3 +3,6 @@ grid.appName: "test" thrall.kinesis.stream.name: "not-used" thrall.kinesis.lowPriorityStream.name: "not-used" domain.root: "notused.example.com" +s3.image.bucket.name: images +s3.image.bucket.endpoint: some-providers-s3-endpoint +s3.thumb.bucket: thumbs From 7307530923f7fe928d478a3db2c72e38ac57f5f1 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Fri, 17 Jan 2025 18:12:11 +0000 Subject: [PATCH 07/39] Ingest and rejected buckets have configurable end points. --- .../mediaservice/lib/config/CommonConfig.scala | 16 +++++++++++++--- image-loader/app/lib/ImageLoaderStore.scala | 2 -- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala index b95e98a6c1..340e6473da 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala @@ -53,8 +53,18 @@ abstract class CommonConfig(resources: GridConfigResources) extends AwsClientV1B lazy val softDeletedMetadataTable: String = string("dynamo.table.softDelete.metadata") val maybeIngestSqsQueueUrl: Option[String] = stringOpt("sqs.ingest.queue.url") - val maybeIngestBucket: Option[S3Bucket] = stringOpt("s3.ingest.bucket").map(S3Bucket(_, S3.AmazonAwsS3Endpoint)) - val maybeFailBucket: Option[S3Bucket] = stringOpt("s3.fail.bucket").map(S3Bucket(_, S3.AmazonAwsS3Endpoint)) + val maybeIngestBucket: Option[S3Bucket] = for { + ingestBucket <- stringOpt("s3.ingest.bucket.name") + ingestBucketEndpoint <- stringOpt("s3.ingest.bucket.endpoint") + } yield { + S3Bucket(ingestBucket, ingestBucketEndpoint) + } + val maybeFailBucket: Option[S3Bucket] = for { + failBucket <- stringOpt("s3.fail.bucket.name") + failBucketEndpoint <- stringOpt("s3.fail.bucket.endpoint") + } yield { + S3Bucket(failBucket, failBucketEndpoint) + } val maybeQuarantineBucket: Option[S3Bucket] = stringOpt("s3.quarantine.bucket").map(S3Bucket(_, S3.AmazonAwsS3Endpoint)) @@ -73,7 +83,7 @@ abstract class CommonConfig(resources: GridConfigResources) extends AwsClientV1B val services = new SingleHostServices(domainRoot) - val imageBucket: S3Bucket = S3Bucket(string("s3.image.bucket.name") ,stringOpt("s3.image.bucket.endpoint").get) + val imageBucket: S3Bucket = S3Bucket(string("s3.image.bucket.name"), string("s3.image.bucket.endpoint")) val thumbnailBucket: S3Bucket = S3Bucket(string("s3.thumb.bucket"), S3.AmazonAwsS3Endpoint) val thumbnailBucketS3Endpoint: String = S3.AmazonAwsS3Endpoint diff --git a/image-loader/app/lib/ImageLoaderStore.scala b/image-loader/app/lib/ImageLoaderStore.scala index eb66b8d864..0f7975399b 100644 --- a/image-loader/app/lib/ImageLoaderStore.scala +++ b/image-loader/app/lib/ImageLoaderStore.scala @@ -17,8 +17,6 @@ class S3FileDoesNotExistException extends Exception() class ImageLoaderStore(config: ImageLoaderConfig) extends lib.ImageIngestOperations(config.imageBucket, config.thumbnailBucket, config) with GridLogging { - private val imageIngestS3Endpoint = AmazonAwsS3Endpoint - private def handleNotFound[T](key: String)(doWork: => T)(loggingIfNotFound: => Unit): T = { try { doWork From bc2bab16b31221be9729f0cd0bade4422f50f145 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Fri, 24 Jan 2025 17:27:09 +0000 Subject: [PATCH 08/39] Thumbs bucket has configurable end point. --- .../scala/com/gu/mediaservice/lib/config/CommonConfig.scala | 3 +-- common-lib/src/test/resources/application.conf | 3 ++- media-api/test/lib/elasticsearch/Fixtures.scala | 3 ++- rest-lib/src/test/resources/application.conf | 3 ++- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala index 340e6473da..483ba00ea2 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala @@ -84,8 +84,7 @@ abstract class CommonConfig(resources: GridConfigResources) extends AwsClientV1B val services = new SingleHostServices(domainRoot) val imageBucket: S3Bucket = S3Bucket(string("s3.image.bucket.name"), string("s3.image.bucket.endpoint")) - val thumbnailBucket: S3Bucket = S3Bucket(string("s3.thumb.bucket"), S3.AmazonAwsS3Endpoint) - val thumbnailBucketS3Endpoint: String = S3.AmazonAwsS3Endpoint + val thumbnailBucket: S3Bucket = S3Bucket(string("s3.thumb.bucket.name"), string("s3.thumb.bucket.endpoint")) /** * Load in a list of domain metadata specifications from configuration. For example: diff --git a/common-lib/src/test/resources/application.conf b/common-lib/src/test/resources/application.conf index c178da2149..b5c3e1aee2 100644 --- a/common-lib/src/test/resources/application.conf +++ b/common-lib/src/test/resources/application.conf @@ -65,5 +65,6 @@ usageEvents.queue.name="" s3.image.bucket.name="images" s3.image.bucket.endpoint="some-providers-s3-endpoint" -s3.thumb.bucket="thumbs" +s3.thumb.bucket.name="thumbs" +s3.thumb.bucket.endpoint="some-providers-s3-endpoint" diff --git a/media-api/test/lib/elasticsearch/Fixtures.scala b/media-api/test/lib/elasticsearch/Fixtures.scala index 3364ff9242..fed9e13f6c 100644 --- a/media-api/test/lib/elasticsearch/Fixtures.scala +++ b/media-api/test/lib/elasticsearch/Fixtures.scala @@ -37,7 +37,8 @@ trait Fixtures { "es6.url", "s3.image.bucket.name", "s3.image.bucket.endpoint", - "s3.thumb.bucket", + "s3.thumb.bucket.name", + "s3.thumb.bucket.endpoint", "grid.stage", "grid.appName", "instance.service.my", diff --git a/rest-lib/src/test/resources/application.conf b/rest-lib/src/test/resources/application.conf index cfd0579a07..7e55f72bb2 100644 --- a/rest-lib/src/test/resources/application.conf +++ b/rest-lib/src/test/resources/application.conf @@ -5,4 +5,5 @@ thrall.kinesis.lowPriorityStream.name: "not-used" domain.root: "notused.example.com" s3.image.bucket.name: images s3.image.bucket.endpoint: some-providers-s3-endpoint -s3.thumb.bucket: thumbs +s3.thumb.bucket.name: thumbs +s3.thumb.bucket.endpoint: some-providers-s3-endpoint From d791aa6cce8f659c792bfce8dd63dd1269ace5e0 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Fri, 29 Nov 2024 21:12:03 +0000 Subject: [PATCH 09/39] Configure a GCP S3 client and selectively use it based on the bucket endpoint. Not great. Crops require mock S3 config. Presence of Google details on this AWS path not great. Suggests client might be a component of bucket. --- .../com/gu/mediaservice/lib/aws/S3.scala | 34 ++++++++++++++++--- .../lib/config/CommonConfig.scala | 3 ++ cropper/test/lib/CropsTest.scala | 13 +++++-- 3 files changed, 42 insertions(+), 8 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala index fddaa62c97..4bb49e488c 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala @@ -1,5 +1,7 @@ package com.gu.mediaservice.lib.aws +import com.amazonaws.auth.{AWSStaticCredentialsProvider, BasicAWSCredentials} +import com.amazonaws.client.builder.AwsClientBuilder.EndpointConfiguration import com.amazonaws.services.s3.model._ import com.amazonaws.services.s3.{AmazonS3, AmazonS3ClientBuilder, model} import com.amazonaws.util.IOUtils @@ -68,10 +70,17 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with val AmazonAwsS3Endpoint: String = S3.AmazonAwsS3Endpoint private lazy val amazonS3: AmazonS3 = S3Ops.buildS3Client(config) + private val googleS3: Option[AmazonS3] = S3Ops.buildGoogleS3Client(config) private def clientFor(bucket: S3Bucket): AmazonS3 = { - logger.info("Client for: " + bucket.endpoint) - amazonS3 + (bucket.endpoint match { + case "storage.googleapis.com" => + googleS3 + case _ => + Some(amazonS3) + }).getOrElse { + amazonS3 + } } def signUrl(bucket: S3Bucket, url: URI, image: Image, expiration: DateTime = cachableExpiration(), imageType: ImageFileType = Source): String = { @@ -240,9 +249,24 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with } object S3Ops { - // TODO make this localstack friendly - // TODO: Make this region aware - i.e. RegionUtils.getRegion(region).getServiceEndpoint(AmazonS3.ENDPOINT_PREFIX) - val s3Endpoint = "s3.amazonaws.com" + def buildGoogleS3Client(config: CommonConfig): Option[AmazonS3] = { + config.googleS3AccessKey.flatMap { accessKey => + config.googleS3SecretKey.map { secretKey => + val endpointConfig = new EndpointConfiguration("https://storage.googleapis.com", null) + // create credentials provider + val credentials = new BasicAWSCredentials(accessKey, secretKey) + val credentialsProvider = new AWSStaticCredentialsProvider(credentials) + // create a client config + val clientConfig = new ClientConfiguration() + + val clientBuilder = AmazonS3ClientBuilder.standard() + clientBuilder.setEndpointConfiguration(endpointConfig) + clientBuilder.withCredentials(credentialsProvider) + clientBuilder.withClientConfiguration(clientConfig) + clientBuilder.build() + } + } + } def buildS3Client(config: CommonConfig, localstackAware: Boolean = true, maybeRegionOverride: Option[String] = None): AmazonS3 = { val builder = config.awsLocalEndpoint match { diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala index 483ba00ea2..df7b9d3a10 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala @@ -86,6 +86,9 @@ abstract class CommonConfig(resources: GridConfigResources) extends AwsClientV1B val imageBucket: S3Bucket = S3Bucket(string("s3.image.bucket.name"), string("s3.image.bucket.endpoint")) val thumbnailBucket: S3Bucket = S3Bucket(string("s3.thumb.bucket.name"), string("s3.thumb.bucket.endpoint")) + val googleS3AccessKey = stringOpt("s3.accessKey") + val googleS3SecretKey = stringOpt("s3.secretKey") + /** * Load in a list of domain metadata specifications from configuration. For example: * {{{ diff --git a/cropper/test/lib/CropsTest.scala b/cropper/test/lib/CropsTest.scala index 02752267c0..2c28ba1661 100644 --- a/cropper/test/lib/CropsTest.scala +++ b/cropper/test/lib/CropsTest.scala @@ -1,8 +1,9 @@ package lib -import com.gu.mediaservice.lib.aws.S3Bucket +import com.gu.mediaservice.lib.aws.{S3, S3Bucket} import com.gu.mediaservice.lib.imaging.ImageOperations import com.gu.mediaservice.model._ +import org.mockito.Mockito.when import org.scalatest.funspec.AnyFunSpec import org.scalatest.matchers.should.Matchers import org.scalatestplus.mockito.MockitoSugar @@ -45,13 +46,19 @@ class CropsTest extends AnyFunSpec with Matchers with MockitoSugar { Crops.cropType(Tiff, "TrueColor", hasAlpha = false) shouldBe Jpeg } - private val config = mock[CropperConfig] + private val config = { + val mockConfig = mock[CropperConfig] + when(mockConfig.awsRegion).thenReturn("eu-west-1") + when(mockConfig.googleS3AccessKey).thenReturn(None) + when(mockConfig.googleS3SecretKey).thenReturn(None) + mockConfig + } private val store = mock[CropStore] private val imageOperations: ImageOperations = mock[ImageOperations] private val source: SourceImage = SourceImage("test", mock[Asset], valid = true, mock[ImageMetadata], mock[FileMetadata]) private val bounds: Bounds = Bounds(10, 20, 30, 40) private val outputWidth = 1234 - private val imageBucket = S3Bucket("crops-bucket", endpoint = "some-providers-s3-endpoint") + private val imageBucket = S3Bucket("crops-bucket", S3.AmazonAwsS3Endpoint) it("should should construct a correct address for a master jpg") { val outputFilename = new Crops(config, store, imageOperations, imageBucket) From 542539a8447333578dc2d721afdc3079eed97243 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sat, 30 Nov 2024 10:00:19 +0000 Subject: [PATCH 10/39] v2 sigs on all AWS buckets for better caching. From 11217d43b2c75aae3917562102623f15eb4e47c9 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sat, 23 May 2026 18:55:38 +0100 Subject: [PATCH 11/39] Setup a local Minio S3 client. Use single host URLs for local S3. # Conflicts: # common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala --- .../com/gu/mediaservice/lib/aws/S3.scala | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala index 4bb49e488c..6820b08b97 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala @@ -71,11 +71,14 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with private lazy val amazonS3: AmazonS3 = S3Ops.buildS3Client(config) private val googleS3: Option[AmazonS3] = S3Ops.buildGoogleS3Client(config) + private val localS3: Option[AmazonS3] = S3Ops.buildLocalS3Client(config) private def clientFor(bucket: S3Bucket): AmazonS3 = { (bucket.endpoint match { case "storage.googleapis.com" => googleS3 + case "10.0.45.121:32090" => + localS3 case _ => Some(amazonS3) }).getOrElse { @@ -268,6 +271,26 @@ object S3Ops { } } + def buildLocalS3Client(config: CommonConfig): Option[AmazonS3] = { + config.googleS3AccessKey.flatMap { accessKey => + config.googleS3SecretKey.map { secretKey => + val endpointConfig = new EndpointConfiguration("http://10.0.45.121:32090", null) + // create credentials provider + val credentials = new BasicAWSCredentials(accessKey, secretKey) + val credentialsProvider = new AWSStaticCredentialsProvider(credentials) + // create a client config + val clientConfig = new ClientConfiguration() + + val clientBuilder = AmazonS3ClientBuilder.standard() + clientBuilder.setEndpointConfiguration(endpointConfig) + clientBuilder.withCredentials(credentialsProvider) + clientBuilder.withClientConfiguration(clientConfig) + clientBuilder.withPathStyleAccessEnabled(true) + clientBuilder.build() + } + } + } + def buildS3Client(config: CommonConfig, localstackAware: Boolean = true, maybeRegionOverride: Option[String] = None): AmazonS3 = { val builder = config.awsLocalEndpoint match { case Some(_) if config.isDev => From c30b533d4a49d18c7587416245285397ef9b2f28 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Thu, 23 Jan 2025 23:34:54 +0000 Subject: [PATCH 12/39] Local S3. --- .../src/main/scala/com/gu/mediaservice/lib/aws/S3.scala | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala index 6820b08b97..17b9f8d8ff 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala @@ -20,7 +20,11 @@ case class S3Object(uri: URI, size: Long, metadata: S3Metadata) object S3Object { private def objectUrl(bucket: String, key: String, s3Endpoint: String): URI = { - val bucketUrl = s"$bucket.$s3Endpoint" + val bucketUrl = if (s3Endpoint == "10.0.45.121:32090") { + s"$s3Endpoint/$bucket" + } else { + s"$bucket.$s3Endpoint" + } new URI("http", bucketUrl, s"/$key", null) } From ba390f6fc2aa7249675302d8178a16fe2b8e820b Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Thu, 23 Jan 2025 23:49:29 +0000 Subject: [PATCH 13/39] Local S3. --- .../src/main/scala/com/gu/mediaservice/lib/aws/S3.scala | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala index 17b9f8d8ff..a7575d16c0 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala @@ -21,11 +21,12 @@ case class S3Object(uri: URI, size: Long, metadata: S3Metadata) object S3Object { private def objectUrl(bucket: String, key: String, s3Endpoint: String): URI = { val bucketUrl = if (s3Endpoint == "10.0.45.121:32090") { - s"$s3Endpoint/$bucket" + new URI(s"http://$s3Endpoint/$bucket/$key") } else { - s"$bucket.$s3Endpoint" + val bucketHost = s"$bucket.$s3Endpoint" + new URI("http", bucketHost, s"/$key", null) } - new URI("http", bucketUrl, s"/$key", null) + bucketUrl } def apply(bucket: String, key: String, size: Long, metadata: S3Metadata, s3Endpoint: String): S3Object = From f9d262c7c43175d0bf4e6c440482aac5309f29fd Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Fri, 24 Jan 2025 08:51:33 +0000 Subject: [PATCH 14/39] Local S3 urls to keys. --- .../com/gu/mediaservice/lib/aws/S3.scala | 19 ++++--------------- .../mediaservice/lib/aws/S3KeyFromURL.scala | 17 +++++++++++++++++ cropper/app/lib/CropStore.scala | 11 +++++++---- cropper/app/lib/Crops.scala | 8 +++++--- media-api/app/controllers/MediaApi.scala | 13 +++++++++---- media-api/app/lib/ImageResponse.scala | 14 +++++++------- 6 files changed, 49 insertions(+), 33 deletions(-) create mode 100644 common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3KeyFromURL.scala diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala index a7575d16c0..e607f9d54f 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala @@ -91,10 +91,7 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with } } - def signUrl(bucket: S3Bucket, url: URI, image: Image, expiration: DateTime = cachableExpiration(), imageType: ImageFileType = Source): String = { - // get path and remove leading `/` - val key: Key = url.getPath.drop(1) - + def signUrl(bucket: S3Bucket, key: String, image: Image, expiration: DateTime = cachableExpiration(), imageType: ImageFileType = Source): String = { val contentDisposition = getContentDisposition(image, imageType, config.shortenDownloadFilename) val headers = new ResponseHeaderOverrides().withContentDisposition(contentDisposition) @@ -103,10 +100,7 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with clientFor(bucket).generatePresignedUrl(request).toExternalForm } - def signUrlTony(bucket: S3Bucket, url: URI, expiration: DateTime = cachableExpiration()): URL = { - // get path and remove leading `/` - val key: Key = url.getPath.drop(1) - + def signUrlTony(bucket: S3Bucket, key: String, expiration: DateTime = cachableExpiration()): URL = { val request = new GeneratePresignedUrlRequest(bucket.bucket, key).withExpiration(expiration.toDate) clientFor(bucket).generatePresignedUrl(request) } @@ -136,13 +130,7 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with def doesObjectExist(bucket: S3Bucket, key: String) = { clientFor(bucket).doesObjectExist(bucket.bucket, key) } - - def getObject(bucket: S3Bucket, url: URI): model.S3Object = { // TODO why can't this just be by bucket + key to remove end point knowledge - // get path and remove leading `/` - val key: Key = url.getPath.drop(1) - clientFor(bucket).getObject(new GetObjectRequest(bucket.bucket, key)) - } - + def getObject(bucket: S3Bucket, key: String): model.S3Object = { clientFor(bucket).getObject(new GetObjectRequest(bucket.bucket, key)) } @@ -308,6 +296,7 @@ object S3Ops { config.withAWSCredentials(builder, localstackAware, maybeRegionOverride).build() } + } object S3 { diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3KeyFromURL.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3KeyFromURL.scala new file mode 100644 index 0000000000..a9676e9a53 --- /dev/null +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3KeyFromURL.scala @@ -0,0 +1,17 @@ +package com.gu.mediaservice.lib.aws + +import com.gu.mediaservice.lib.logging.GridLogging + +import java.net.URI + +trait S3KeyFromURL extends GridLogging { + + def keyFromS3URL(bucket: S3Bucket, url: URI): String = { + if (bucket.endpoint == "10.0.45.121:32090") { + url.getPath.drop(bucket.bucket.length + 2) + } else { + url.getPath.drop(1) + } + } + +} diff --git a/cropper/app/lib/CropStore.scala b/cropper/app/lib/CropStore.scala index 60b3a2672e..a3a4b32682 100644 --- a/cropper/app/lib/CropStore.scala +++ b/cropper/app/lib/CropStore.scala @@ -4,10 +4,11 @@ import java.io.File import java.net.{URI, URL} import scala.concurrent.Future import com.gu.mediaservice.lib.S3ImageStorage +import com.gu.mediaservice.lib.aws.S3KeyFromURL import com.gu.mediaservice.lib.logging.LogMarker import com.gu.mediaservice.model._ -class CropStore(config: CropperConfig) extends S3ImageStorage(config) with CropSpecMetadata { +class CropStore(config: CropperConfig) extends S3ImageStorage(config) with CropSpecMetadata with S3KeyFromURL { import com.gu.mediaservice.lib.formatting._ def getSecureCropUri(uri: URI): Option[URL] = @@ -49,9 +50,11 @@ class CropStore(config: CropperConfig) extends S3ImageStorage(config) with CropS date = userMetadata.get("date").flatMap(parseDateTime) dimensions = Dimensions(width, height) + key = keyFromS3URL(config.imgPublishingBucket, s3Object.uri) + sizing = Asset( - signedCropAssetUrl(s3Object.uri), + signedCropAssetUrl(key), Some(s3Object.size), objectMetadata.contentType, Some(dimensions), @@ -82,8 +85,8 @@ class CropStore(config: CropperConfig) extends S3ImageStorage(config) with CropS instance.id + "/" + id } - private def signedCropAssetUrl(uri: URI): URI = { - signUrlTony(config.imgPublishingBucket, uri).toURI + private def signedCropAssetUrl(key: String): URI = { + signUrlTony(config.imgPublishingBucket, key).toURI } } diff --git a/cropper/app/lib/Crops.scala b/cropper/app/lib/Crops.scala index 463fa8e1df..732b4c54ed 100644 --- a/cropper/app/lib/Crops.scala +++ b/cropper/app/lib/Crops.scala @@ -3,7 +3,7 @@ package lib import java.io.File import com.gu.mediaservice.lib.metadata.FileMetadataHelper import com.gu.mediaservice.lib.Files -import com.gu.mediaservice.lib.aws.{S3, S3Bucket} +import com.gu.mediaservice.lib.aws.{S3, S3Bucket, S3KeyFromURL} import com.gu.mediaservice.lib.imaging.{ExportResult, ImageOperations} import com.gu.mediaservice.lib.logging.{GridLogging, LogMarker, Stopwatch} import com.gu.mediaservice.model._ @@ -17,7 +17,7 @@ case object InvalidCropRequest extends Exception("Crop request invalid for image case class MasterCrop(sizing: Future[Asset], file: File, dimensions: Dimensions, aspectRatio: Float) -class Crops(config: CropperConfig, store: CropStore, imageOperations: ImageOperations, imageBucket: S3Bucket)(implicit ec: ExecutionContext) extends GridLogging { +class Crops(config: CropperConfig, store: CropStore, imageOperations: ImageOperations, imageBucket: S3Bucket)(implicit ec: ExecutionContext) extends GridLogging with S3KeyFromURL { import Files._ private val cropQuality = 75d @@ -116,7 +116,9 @@ class Crops(config: CropperConfig, store: CropStore, imageOperations: ImageOpera val hasAlpha = apiImage.fileMetadata.colourModelInformation.get("hasAlpha").flatMap(a => Try(a.toBoolean).toOption).getOrElse(true) val cropType = Crops.cropType(mimeType, colourType, hasAlpha) - val secureUrl = s3.signUrlTony(imageBucket, secureFile) + val key = keyFromS3URL(imageBucket, secureFile) + + val secureUrl = s3.signUrlTony(imageBucket, key) Stopwatch.async(s"making crop assets for ${apiImage.id} ${Crop.getCropId(source.bounds)}") { for { diff --git a/media-api/app/controllers/MediaApi.scala b/media-api/app/controllers/MediaApi.scala index 4d9bfbd573..bac5422571 100644 --- a/media-api/app/controllers/MediaApi.scala +++ b/media-api/app/controllers/MediaApi.scala @@ -11,6 +11,8 @@ import com.gu.mediaservice.lib.auth.provider.ApiKeyAuthenticationProvider import com.gu.mediaservice.lib.aws._ import com.gu.mediaservice.lib.aws.{ContentDisposition, Embedder, ThrallMessageSender, UpdateMessage} import com.gu.mediaservice.lib.aws.{ContentDisposition, S3, ThrallMessageSender, UpdateMessage} +import com.gu.mediaservice.lib.aws.{ContentDisposition, ThrallMessageSender, UpdateMessage} +import com.gu.mediaservice.lib.aws.{ContentDisposition, S3, S3KeyFromURL, ThrallMessageSender, UpdateMessage} import com.gu.mediaservice.lib.config.InstanceForRequest import com.gu.mediaservice.lib.events.UsageEvents import com.gu.mediaservice.lib.formatting.printDateTime @@ -50,7 +52,7 @@ class MediaApi( authorisation: Authorisation, embedder: Embedder, events: UsageEvents, - )(implicit val ec: ExecutionContext) extends BaseController with MessageSubjects with ArgoHelpers with ContentDisposition with InstanceForRequest { +)(implicit val ec: ExecutionContext) extends BaseController with MessageSubjects with ArgoHelpers with ContentDisposition with InstanceForRequest with S3KeyFromURL { private val gridClient: GridClient = GridClient(config.services)(ws) @@ -323,7 +325,8 @@ class MediaApi( val maybeResult = for { export <- source.exports.find(_.id.contains(exportId)) asset <- export.assets.find(_.dimensions.exists(_.width == width)) - s3Object <- Try(s3Client.getObject(config.imgPublishingBucket, asset.file)).toOption + key = keyFromS3URL(config.imgPublishingBucket, asset.file) + s3Object <- Try(s3Client.getObject(config.imgPublishingBucket, key)).toOption file = StreamConverters.fromInputStream(() => s3Object.getObjectContent) entity = HttpEntity.Streamed(file, asset.size, asset.mimeType.map(_.name)) result = Result(ResponseHeader(OK), entity).withHeaders("Content-Disposition" -> getContentDisposition(source, export, asset, config.shortenDownloadFilename)) @@ -452,7 +455,8 @@ class MediaApi( val apiKey = request.user.accessor logger.info(logMarker, s"Download original image: $id from user: ${Authentication.getIdentity(request.user)}") mediaApiMetrics.incrementImageDownload(apiKey, mediaApiMetrics.OriginalDownloadType) - val s3Object = s3Client.getObject(config.imageBucket, image.source.file) + val key = keyFromS3URL(config.imageBucket, image.source.file) + val s3Object = s3Client.getObject(config.imageBucket, key) val file = StreamConverters.fromInputStream(() => s3Object.getObjectContent) val entity = HttpEntity.Streamed(file, image.source.size, image.source.mimeType.map(_.name)) @@ -515,8 +519,9 @@ class MediaApi( logger.info(logMarker, s"Download optimised image: $id from user: ${Authentication.getIdentity(request.user)}") mediaApiMetrics.incrementImageDownload(apiKey, mediaApiMetrics.OptimisedDownloadType) + val key = keyFromS3URL(config.imageBucket, image.optimisedPng.getOrElse(image.source).file) val sourceImageUri = - new URI(s3Client.signUrl(config.imageBucket, image.optimisedPng.getOrElse(image.source).file, image, imageType = image.optimisedPng match { + new URI(s3Client.signUrl(config.imageBucket, key, image, imageType = image.optimisedPng match { case Some(_) => OptimisedPng case _ => Source })) diff --git a/media-api/app/lib/ImageResponse.scala b/media-api/app/lib/ImageResponse.scala index 73824d0169..7cd4184a62 100644 --- a/media-api/app/lib/ImageResponse.scala +++ b/media-api/app/lib/ImageResponse.scala @@ -3,6 +3,7 @@ package lib import com.gu.mediaservice.lib.argo.model._ import com.gu.mediaservice.lib.auth.{Internal, Tier} import com.gu.mediaservice.lib.aws.S3 +import com.gu.mediaservice.lib.aws.S3KeyFromURL import com.gu.mediaservice.lib.collections.CollectionsManager import com.gu.mediaservice.lib.logging.{GridLogging, LogMarker} import com.gu.mediaservice.model._ @@ -12,17 +13,15 @@ import lib.ImageResponse.extractAliasFieldValues import lib.elasticsearch.SourceWrapper import lib.usagerights.CostCalculator import org.apache.commons.codec.binary.Base64 -import org.joda.time.DateTime -import play.api.libs.functional.syntax._ import play.api.libs.json._ import play.utils.UriEncoding -import java.net.{URI, URLEncoder} +import java.net.URI import scala.annotation.tailrec import scala.util.{Failure, Try} class ImageResponse(config: MediaApiConfig, s3Client: S3, usageQuota: UsageQuota) - extends EditsResponse with GridLogging { + extends EditsResponse with GridLogging with S3KeyFromURL { implicit val usageQuotas: UsageQuota = usageQuota @@ -79,11 +78,12 @@ class ImageResponse(config: MediaApiConfig, s3Client: S3, usageQuota: UsageQuota val fileUri = image.source.file - val imageUrl = s3Client.signUrl(config.imageBucket, fileUri, image, imageType = Source) + val key = keyFromS3URL(config.imageBucket, fileUri) + val imageUrl = s3Client.signUrl(config.imageBucket, key, image, imageType = Source) val pngUrl: Option[String] = pngFileUri - .map(s3Client.signUrl(config.imageBucket, _, image, imageType = OptimisedPng)) + .map(uri => s3Client.signUrl(config.imageBucket, keyFromS3URL(config.imageBucket, uri), image, imageType = OptimisedPng)) - def s3SignedThumbUrl = s3Client.signUrl(config.thumbnailBucket, fileUri, image, imageType = Thumbnail) + def s3SignedThumbUrl = s3Client.signUrl(config.thumbnailBucket, key, image, imageType = Thumbnail) val thumbUrl = config.cloudFrontDomainThumbBucket .map(domain => s"https://$domain${fileUri.getPath}") From 88a1179da9c62ded6003221f223543c0818ab38a Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Fri, 24 Jan 2025 22:13:08 +0000 Subject: [PATCH 15/39] Local S3 - host. --- .../src/main/scala/com/gu/mediaservice/lib/aws/S3.scala | 8 ++++---- .../scala/com/gu/mediaservice/lib/aws/S3KeyFromURL.scala | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala index e607f9d54f..21e74d2e54 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala @@ -20,7 +20,7 @@ case class S3Object(uri: URI, size: Long, metadata: S3Metadata) object S3Object { private def objectUrl(bucket: String, key: String, s3Endpoint: String): URI = { - val bucketUrl = if (s3Endpoint == "10.0.45.121:32090") { + val bucketUrl = if (s3Endpoint == "minio.griddev.eelpieconsulting.co.uk") { new URI(s"http://$s3Endpoint/$bucket/$key") } else { val bucketHost = s"$bucket.$s3Endpoint" @@ -82,7 +82,7 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with (bucket.endpoint match { case "storage.googleapis.com" => googleS3 - case "10.0.45.121:32090" => + case "minio.griddev.eelpieconsulting.co.uk" => localS3 case _ => Some(amazonS3) @@ -130,7 +130,7 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with def doesObjectExist(bucket: S3Bucket, key: String) = { clientFor(bucket).doesObjectExist(bucket.bucket, key) } - + def getObject(bucket: S3Bucket, key: String): model.S3Object = { clientFor(bucket).getObject(new GetObjectRequest(bucket.bucket, key)) } @@ -267,7 +267,7 @@ object S3Ops { def buildLocalS3Client(config: CommonConfig): Option[AmazonS3] = { config.googleS3AccessKey.flatMap { accessKey => config.googleS3SecretKey.map { secretKey => - val endpointConfig = new EndpointConfiguration("http://10.0.45.121:32090", null) + val endpointConfig = new EndpointConfiguration("https://minio.griddev.eelpieconsulting.co.uk", null) // create credentials provider val credentials = new BasicAWSCredentials(accessKey, secretKey) val credentialsProvider = new AWSStaticCredentialsProvider(credentials) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3KeyFromURL.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3KeyFromURL.scala index a9676e9a53..f1ecb8cbe7 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3KeyFromURL.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3KeyFromURL.scala @@ -7,7 +7,7 @@ import java.net.URI trait S3KeyFromURL extends GridLogging { def keyFromS3URL(bucket: S3Bucket, url: URI): String = { - if (bucket.endpoint == "10.0.45.121:32090") { + if (bucket.endpoint == "minio.griddev.eelpieconsulting.co.uk") { url.getPath.drop(bucket.bucket.length + 2) } else { url.getPath.drop(1) From 3fea9aed1fef02495632d2425f4413edbc998e96 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sat, 25 Jan 2025 11:50:27 +0000 Subject: [PATCH 16/39] Kahuna UI can connect to ingest bucket endpoint. --- kahuna/app/lib/KahunaConfig.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kahuna/app/lib/KahunaConfig.scala b/kahuna/app/lib/KahunaConfig.scala index 525423ed05..721b79e199 100644 --- a/kahuna/app/lib/KahunaConfig.scala +++ b/kahuna/app/lib/KahunaConfig.scala @@ -51,7 +51,7 @@ class KahunaConfig(resources: GridConfigResources) extends CommonConfig(resource val frameAncestors: Set[String] = getStringSet("security.frameAncestors") val connectSources: Set[String] = getStringSet("security.connectSources") ++ maybeBucketForUIUploads.map { bucket => if (isDev) "https://localstack.media.local.dev-gutools.co.uk" - else s"https://$bucket.s3.$awsRegion.amazonaws.com" + else s"https://$bucket.s3.$awsRegion.amazonaws.com" // TODO push to bucket end point } ++ telemetryUri val fontSources: Set[String] = getStringSet("security.fontSources") val imageSources: Set[String] = getStringSet("security.imageSources") From 94076e5cf39720cf56af5a7928d612fd4509bb4c Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Wed, 2 Apr 2025 21:31:33 +0100 Subject: [PATCH 17/39] Noisy log. --- image-loader/app/controllers/ImageLoaderController.scala | 2 -- 1 file changed, 2 deletions(-) diff --git a/image-loader/app/controllers/ImageLoaderController.scala b/image-loader/app/controllers/ImageLoaderController.scala index a350a200b2..e2c546da4c 100644 --- a/image-loader/app/controllers/ImageLoaderController.scala +++ b/image-loader/app/controllers/ImageLoaderController.scala @@ -141,8 +141,6 @@ class ImageLoaderController(auth: Authentication, } private def handleMessageFromIngestBucket(sqsMessage: SQSMessage)(basicLogMarker: LogMarker): Future[Unit] = { - logger.info(basicLogMarker, sqsMessage.toString) - extractS3KeyFromSqsMessage(sqsMessage) match { case Failure(exception) => metrics.failedIngestsFromQueue.increment() From caed55573013cf59ff7140a0c482b01d44561832 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Wed, 2 Apr 2025 21:32:05 +0100 Subject: [PATCH 18/39] Noisy log Redact debug. --- image-loader/app/lib/imaging/FileMetadataReader.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/image-loader/app/lib/imaging/FileMetadataReader.scala b/image-loader/app/lib/imaging/FileMetadataReader.scala index f81b9ce3a7..40b9ad413e 100644 --- a/image-loader/app/lib/imaging/FileMetadataReader.scala +++ b/image-loader/app/lib/imaging/FileMetadataReader.scala @@ -125,7 +125,7 @@ object FileMetadataReader extends GridLogging { val redactionReplacementValue = s"REDACTED (value longer than $redactionThreshold characters, please refer to the metadata stored in the file itself)" private def redactLongFieldValues(imageId: String, metadataType: String, exceptions: List[String] = Nil)(props: Map[String, String]) = props.map { case (fieldName, value) if value.length > redactionThreshold && !exceptions.exists(fieldName.contains) => - logger.warn(s"Redacting '$fieldName' $metadataType field for image $imageId, as it's problematically long (longer than $redactionThreshold characters") + logger.debug(s"Redacting '$fieldName' $metadataType field for image $imageId, as it's problematically long (longer than $redactionThreshold characters") fieldName -> redactionReplacementValue case keyValuePair => keyValuePair } From b53273def940c705bf3ef9d3279046543b350146 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 13 Apr 2025 17:17:46 +0100 Subject: [PATCH 19/39] GCP S3 interop does not support S3 bulkDelete; reimplement as single calls. --- .../lib/ImageIngestOperations.scala | 49 +++++++++++++------ 1 file changed, 35 insertions(+), 14 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala index f1e6a0f26a..7c4606ca7a 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala @@ -59,22 +59,43 @@ class ImageIngestOperations(imageBucket: S3Bucket, thumbnailBucket: S3Bucket, co private def bulkDelete(bucket: S3Bucket, keys: List[String]): Future[Map[String, Boolean]] = keys match { case Nil => Future.successful(Map.empty) - case _ => Future { - try { - logger.info(s"Creating S3 bulkDelete request for $bucket / keys: " + keys.mkString(",")) - deleteObjects(bucket, keys) - keys.map { key => - key -> true - }.toMap - } catch { - case partialFailure: MultiObjectDeleteException => - logger.warn(s"Partial failure when deleting images from $bucket: ${partialFailure.getMessage} ${partialFailure.getErrors}") - val errorKeys = partialFailure.getErrors.asScala.map(_.getKey).toSet + case _ => + val bulkDeleteImplemented = bucket.endpoint != "storage.googleapis.com" + if (bulkDeleteImplemented) { + Future { + try { + logger.info(s"Bulk deleting S3 objects from $bucket: " + keys.mkString(",")) + deleteObjects(bucket, keys) + keys.map { key => + key -> true + }.toMap + } catch { + case partialFailure: MultiObjectDeleteException => + logger.warn(s"Partial failure when deleting images from $bucket: ${partialFailure.getMessage} ${partialFailure.getErrors}") + val errorKeys = partialFailure.getErrors.asScala.map(_.getKey).toSet + keys.map { key => + key -> !errorKeys.contains(key) + }.toMap + } + } + + } else { + Future.sequence { keys.map { key => - key -> !errorKeys.contains(key) - }.toMap + Future { + logger.info(s"Deleting S3 objects from $bucket: " + key) + try { + val x = deleteObject(bucket, key) + (key, true) + } catch { + case e: Exception => + logger.warn(s"Failure when deleting images from $bucket: $key, ${e.getMessage}") + (key, false) + } + } + } + }.map(_.toMap) } - } } def deleteOriginal(id: String)(implicit logMarker: LogMarker, instance: Instance): Future[Unit] = if(isVersionedS3) deleteVersionedImage(imageBucket, fileKeyFromId(id)) else deleteImage(imageBucket, fileKeyFromId(id)) From eb1da7e3c1a7afd535c15c460b1c46c4df75da79 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Mon, 28 Apr 2025 21:07:58 +0100 Subject: [PATCH 20/39] Logging tidy up; storeImage --- .../src/main/scala/com/gu/mediaservice/lib/S3ImageStorage.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/S3ImageStorage.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/S3ImageStorage.scala index 54b17dba01..a4d603f1a6 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/S3ImageStorage.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/S3ImageStorage.scala @@ -17,7 +17,7 @@ class S3ImageStorage(config: CommonConfig) extends S3(config) with ImageStorage def storeImage(bucket: S3Bucket, id: String, file: File, mimeType: Option[MimeType], meta: Map[String, String] = Map.empty, overwrite: Boolean) (implicit logMarker: LogMarker) = { - logger.info(logMarker, s"bucket: $bucket, id: $id, meta: $meta") + logger.info(logMarker, s"storeImage to bucket: ${bucket.bucket}, id: $id, meta: $meta") val eventualObject = if (overwrite) { store(bucket, id, file, mimeType, meta, cacheSetting) } else { From bcb6526fbbd54c0ef576789bbae39a2ef5080fa4 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Mon, 28 Apr 2025 21:11:15 +0100 Subject: [PATCH 21/39] Logging tidy up; thumbnail bucket. --- .../scala/com/gu/mediaservice/lib/ImageIngestOperations.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala index 7c4606ca7a..04a1de029d 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala @@ -44,7 +44,7 @@ class ImageIngestOperations(imageBucket: S3Bucket, thumbnailBucket: S3Bucket, co private def storeThumbnailImage(storableImage: StorableThumbImage) (implicit logMarker: LogMarker): Future[S3Object] = { val instanceSpecificKey = instanceAwareThumbnailImageKey(storableImage) - logger.info(s"Storing thumbnail to instance specific key: $thumbnailBucket / $instanceSpecificKey") + logger.info(s"Storing thumbnail to instance specific key: ${thumbnailBucket.bucket} / $instanceSpecificKey") storeImage(thumbnailBucket, instanceSpecificKey, storableImage.file, Some(storableImage.mimeType), overwrite = true) } From bd0a1dc954cd38e31948e669d59cbba738a9bb88 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Mon, 28 Apr 2025 21:17:05 +0100 Subject: [PATCH 22/39] Clean up; unused unit assignment. --- .../scala/com/gu/mediaservice/lib/ImageIngestOperations.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala index 04a1de029d..f2758567d1 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala @@ -85,7 +85,7 @@ class ImageIngestOperations(imageBucket: S3Bucket, thumbnailBucket: S3Bucket, co Future { logger.info(s"Deleting S3 objects from $bucket: " + key) try { - val x = deleteObject(bucket, key) + deleteObject(bucket, key) (key, true) } catch { case e: Exception => From 0727f029db4a8ae523ac1614aa91d7eaba99f6f0 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Mon, 28 Apr 2025 21:17:47 +0100 Subject: [PATCH 23/39] Logging; lower to debug. --- .../scala/com/gu/mediaservice/lib/ImageIngestOperations.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala index f2758567d1..0f65f4e7bb 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala @@ -89,7 +89,7 @@ class ImageIngestOperations(imageBucket: S3Bucket, thumbnailBucket: S3Bucket, co (key, true) } catch { case e: Exception => - logger.warn(s"Failure when deleting images from $bucket: $key, ${e.getMessage}") + logger.debug(s"Failure when deleting images from $bucket: $key, ${e.getMessage}") (key, false) } } From f698542c5c9093094fd96cb0f3c4aa76b5d8c5e7 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Mon, 28 Apr 2025 21:20:17 +0100 Subject: [PATCH 24/39] Logging; bucket name. --- .../scala/com/gu/mediaservice/lib/ImageIngestOperations.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala index 0f65f4e7bb..d4107b36f0 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala @@ -64,7 +64,7 @@ class ImageIngestOperations(imageBucket: S3Bucket, thumbnailBucket: S3Bucket, co if (bulkDeleteImplemented) { Future { try { - logger.info(s"Bulk deleting S3 objects from $bucket: " + keys.mkString(",")) + logger.info(s"Bulk deleting S3 objects from ${bucket.bucket}: " + keys.mkString(",")) deleteObjects(bucket, keys) keys.map { key => key -> true From 8a80abf4865b0387b8e5d26585eb1367e24fecca Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Mon, 28 Apr 2025 21:21:09 +0100 Subject: [PATCH 25/39] Logging; bucket name. --- .../scala/com/gu/mediaservice/lib/ImageIngestOperations.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala index d4107b36f0..38c5914746 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala @@ -83,7 +83,7 @@ class ImageIngestOperations(imageBucket: S3Bucket, thumbnailBucket: S3Bucket, co Future.sequence { keys.map { key => Future { - logger.info(s"Deleting S3 objects from $bucket: " + key) + logger.info(s"Deleting S3 objects from ${bucket.bucket}: " + key) try { deleteObject(bucket, key) (key, true) From 6a40195ccd8915c248b076e3e820d6e27fd8d9a4 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sat, 7 Feb 2026 17:32:56 +0000 Subject: [PATCH 26/39] Inject S3 into Crops from CropperComponents. Consistent with all other services. --- cropper/app/CropperComponents.scala | 4 +++- cropper/app/lib/Crops.scala | 4 +--- cropper/test/lib/CropsTest.scala | 9 +++++---- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/cropper/app/CropperComponents.scala b/cropper/app/CropperComponents.scala index 509192dedc..d570859ade 100644 --- a/cropper/app/CropperComponents.scala +++ b/cropper/app/CropperComponents.scala @@ -1,4 +1,5 @@ import com.gu.mediaservice.GridClient +import com.gu.mediaservice.lib.aws.S3 import com.gu.mediaservice.lib.imaging.ImageOperations import com.gu.mediaservice.lib.management.Management import com.gu.mediaservice.lib.play.GridComponents @@ -12,8 +13,9 @@ class CropperComponents(context: Context) extends GridComponents(context, new Cr val store = new CropStore(config) val imageOperations = new ImageOperations(context.environment.rootPath.getAbsolutePath) + val s3 = new S3(config) - val crops = new Crops(config, store, imageOperations, config.imageBucket) + val crops = new Crops(config, store, imageOperations, config.imageBucket, s3) val notifications = new Notifications(config) private val gridClient = GridClient(config.services)(wsClient) diff --git a/cropper/app/lib/Crops.scala b/cropper/app/lib/Crops.scala index 732b4c54ed..817b0356ce 100644 --- a/cropper/app/lib/Crops.scala +++ b/cropper/app/lib/Crops.scala @@ -17,7 +17,7 @@ case object InvalidCropRequest extends Exception("Crop request invalid for image case class MasterCrop(sizing: Future[Asset], file: File, dimensions: Dimensions, aspectRatio: Float) -class Crops(config: CropperConfig, store: CropStore, imageOperations: ImageOperations, imageBucket: S3Bucket)(implicit ec: ExecutionContext) extends GridLogging with S3KeyFromURL { +class Crops(config: CropperConfig, store: CropStore, imageOperations: ImageOperations, imageBucket: S3Bucket, s3: S3)(implicit ec: ExecutionContext) extends GridLogging with S3KeyFromURL { import Files._ private val cropQuality = 75d @@ -26,8 +26,6 @@ class Crops(config: CropperConfig, store: CropStore, imageOperations: ImageOpera // We don't overly care about output crop file sizes here, but prefer a fast output, so turn it right down. private val pngCropQuality = 1d - private val s3 = new S3(config) - def outputFilename(source: SourceImage, bounds: Bounds, outputWidth: Int, fileType: MimeType, isMaster: Boolean = false)(implicit instance: Instance): String = { val masterString: String = if (isMaster) "master/" else "" instance.id + "/" + s"${source.id}/${Crop.getCropId(bounds)}/$masterString$outputWidth${fileType.fileExtension}" diff --git a/cropper/test/lib/CropsTest.scala b/cropper/test/lib/CropsTest.scala index 2c28ba1661..52cb6f82b7 100644 --- a/cropper/test/lib/CropsTest.scala +++ b/cropper/test/lib/CropsTest.scala @@ -59,24 +59,25 @@ class CropsTest extends AnyFunSpec with Matchers with MockitoSugar { private val bounds: Bounds = Bounds(10, 20, 30, 40) private val outputWidth = 1234 private val imageBucket = S3Bucket("crops-bucket", S3.AmazonAwsS3Endpoint) + private val s3 = new S3(config) it("should should construct a correct address for a master jpg") { - val outputFilename = new Crops(config, store, imageOperations, imageBucket) + val outputFilename = new Crops(config, store, imageOperations, imageBucket, s3) .outputFilename(source, bounds, outputWidth, Jpeg, isMaster = true) outputFilename shouldBe "an-instance/test/10_20_30_40/master/1234.jpg" } it("should should construct a correct address for a non-master jpg") { - val outputFilename = new Crops(config, store, imageOperations, imageBucket) + val outputFilename = new Crops(config, store, imageOperations, imageBucket, s3) .outputFilename(source, bounds, outputWidth, Jpeg) outputFilename shouldBe "an-instance/test/10_20_30_40/1234.jpg" } it("should should construct a correct address for a non-master tiff") { - val outputFilename = new Crops(config, store, imageOperations, imageBucket) + val outputFilename = new Crops(config, store, imageOperations, imageBucket, s3) .outputFilename(source, bounds, outputWidth, Tiff) outputFilename shouldBe "an-instance/test/10_20_30_40/1234.tiff" } it("should should construct a correct address for a non-master png") { - val outputFilename = new Crops(config, store, imageOperations, imageBucket) + val outputFilename = new Crops(config, store, imageOperations, imageBucket, s3) .outputFilename(source, bounds, outputWidth, Png) outputFilename shouldBe "an-instance/test/10_20_30_40/1234.png" } From 80c0a7d1deebda2823fc047ad2f0be32848518c8 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sat, 23 May 2026 19:03:13 +0100 Subject: [PATCH 27/39] S3Object takes a full S3Bucket as it's bucket parameter. endpoint parameter folds into this. --- .../lib/ImageIngestOperations.scala | 15 +++++------ .../com/gu/mediaservice/lib/aws/S3.scala | 25 ++++++++++--------- .../test/scala/model/ImageUploadTest.scala | 2 +- 3 files changed, 20 insertions(+), 22 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala index 38c5914746..c158008cfd 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala @@ -127,37 +127,34 @@ sealed trait ImageWrapper { } sealed trait StorableImage extends ImageWrapper { def toProjectedS3Object(thumbBucket: S3Bucket): S3Object = S3Object( - thumbBucket.bucket, + thumbBucket, ImageIngestOperations.fileKeyFromId(id)(instance), file, Some(mimeType), lastModified = None, - meta, - s3Endpoint = thumbBucket.endpoint + meta ) } case class StorableThumbImage(id: String, file: File, mimeType: MimeType, meta: Map[String, String] = Map.empty, instance: Instance) extends StorableImage case class StorableOriginalImage(id: String, file: File, mimeType: MimeType, lastModified: DateTime, meta: Map[String, String] = Map.empty, instance: Instance) extends StorableImage { override def toProjectedS3Object(thumbBucket: S3Bucket): S3Object = S3Object( - thumbBucket.bucket, + thumbBucket, ImageIngestOperations.fileKeyFromId(id)(instance), file, Some(mimeType), lastModified = Some(lastModified), - meta, - s3Endpoint = thumbBucket.endpoint + meta ) } case class StorableOptimisedImage(id: String, file: File, mimeType: MimeType, meta: Map[String, String] = Map.empty, instance: Instance) extends StorableImage { override def toProjectedS3Object(thumbBucket: S3Bucket): S3Object = S3Object( - thumbBucket.bucket, + thumbBucket, ImageIngestOperations.optimisedPngKeyFromId(id)(instance), file, Some(mimeType), lastModified = None, - meta = meta, - s3Endpoint = thumbBucket.endpoint + meta = meta ) } diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala index 21e74d2e54..30bfa89b0e 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala @@ -19,21 +19,23 @@ import scala.jdk.CollectionConverters._ case class S3Object(uri: URI, size: Long, metadata: S3Metadata) object S3Object { - private def objectUrl(bucket: String, key: String, s3Endpoint: String): URI = { + private def objectUrl(bucket: S3Bucket, key: String): URI = { + val s3Endpoint = bucket.endpoint + val bucketName = bucket.bucket val bucketUrl = if (s3Endpoint == "minio.griddev.eelpieconsulting.co.uk") { - new URI(s"http://$s3Endpoint/$bucket/$key") + new URI(s"http://$s3Endpoint/$bucketName/$key") } else { - val bucketHost = s"$bucket.$s3Endpoint" + val bucketHost = s"$bucketName.$s3Endpoint" new URI("http", bucketHost, s"/$key", null) } bucketUrl } - def apply(bucket: String, key: String, size: Long, metadata: S3Metadata, s3Endpoint: String): S3Object = - apply(objectUrl(bucket, key, s3Endpoint), size, metadata) + def apply(bucket: S3Bucket, key: String, size: Long, metadata: S3Metadata): S3Object = + apply(objectUrl(bucket, key), size, metadata) - def apply(bucket: String, key: String, file: File, mimeType: Option[MimeType], lastModified: Option[DateTime], - meta: Map[String, String] = Map.empty, cacheControl: Option[String] = None, s3Endpoint: String): S3Object = { + def apply(bucket: S3Bucket, key: String, file: File, mimeType: Option[MimeType], lastModified: Option[DateTime], + meta: Map[String, String] = Map.empty, cacheControl: Option[String] = None): S3Object = { S3Object( bucket, key, @@ -45,8 +47,7 @@ object S3Object { cacheControl, lastModified ) - ), - s3Endpoint + ) ) } } @@ -195,7 +196,7 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with client.putObject(req) // once we've completed the PUT read back to ensure that we are returning reality val metadata = client.getObjectMetadata(bucket.bucket, id) - S3Object(bucket.bucket, id, metadata.getContentLength, S3Metadata(metadata), bucket.endpoint) + S3Object(bucket, id, metadata.getContentLength, S3Metadata(metadata)) }(markers) } @@ -209,7 +210,7 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with }.flatMap { case Some(objectMetadata) => logger.info(logMarker, s"Skipping storing of S3 file $id as key is already present in bucket $bucket") - Future.successful(S3Object(bucket.bucket, id, objectMetadata.getContentLength, S3Metadata(objectMetadata), bucket.endpoint)) + Future.successful(S3Object(bucket, id, objectMetadata.getContentLength, S3Metadata(objectMetadata))) case None => store(bucket, id, file, mimeType, meta, cacheControl) } @@ -223,7 +224,7 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with val summaries = listing.getObjectSummaries.asScala summaries.map(summary => (summary.getKey, summary)).foldLeft(List[S3Object]()) { case (memo: List[S3Object], (key: String, summary: S3ObjectSummary)) => - S3Object(bucket.bucket, key, summary.getSize, getMetadata(bucket, key), bucket.endpoint) :: memo + S3Object(bucket, key, summary.getSize, getMetadata(bucket, key)) :: memo } } diff --git a/image-loader/test/scala/model/ImageUploadTest.scala b/image-loader/test/scala/model/ImageUploadTest.scala index 6dc4674240..6c9bd823c2 100644 --- a/image-loader/test/scala/model/ImageUploadTest.scala +++ b/image-loader/test/scala/model/ImageUploadTest.scala @@ -57,7 +57,7 @@ class ImageUploadTest extends AsyncFunSuite with Matchers with MockitoSugar { def mockStore = (a: StorableImage) => Future.successful( - S3Object("madeupname", "madeupkey", a.file, Some(a.mimeType), None, a.meta, None, S3.AmazonAwsS3Endpoint) + S3Object(S3Bucket("madeupname", S3.AmazonAwsS3Endpoint), "madeupkey", a.file, Some(a.mimeType), None, a.meta, None) ) def storeOrProjectOriginalFile: StorableOriginalImage => Future[S3Object] = mockStore From db3418d791daf143ad8ac70b94d2ae0ecb210a2b Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 14 Sep 2025 12:55:36 +0100 Subject: [PATCH 28/39] S3 objectUrl for bucket and key can move to the bucket (which knows it's own URL scheme). --- .../scala/com/gu/mediaservice/lib/aws/S3.scala | 13 +------------ .../com/gu/mediaservice/lib/aws/S3Bucket.scala | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala index 30bfa89b0e..c6c44037b9 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala @@ -19,20 +19,9 @@ import scala.jdk.CollectionConverters._ case class S3Object(uri: URI, size: Long, metadata: S3Metadata) object S3Object { - private def objectUrl(bucket: S3Bucket, key: String): URI = { - val s3Endpoint = bucket.endpoint - val bucketName = bucket.bucket - val bucketUrl = if (s3Endpoint == "minio.griddev.eelpieconsulting.co.uk") { - new URI(s"http://$s3Endpoint/$bucketName/$key") - } else { - val bucketHost = s"$bucketName.$s3Endpoint" - new URI("http", bucketHost, s"/$key", null) - } - bucketUrl - } def apply(bucket: S3Bucket, key: String, size: Long, metadata: S3Metadata): S3Object = - apply(objectUrl(bucket, key), size, metadata) + apply(bucket.objectUrl(key), size, metadata) def apply(bucket: S3Bucket, key: String, file: File, mimeType: Option[MimeType], lastModified: Option[DateTime], meta: Map[String, String] = Map.empty, cacheControl: Option[String] = None): S3Object = { diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala index 2fc155c89c..6b79669091 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala @@ -1,3 +1,18 @@ package com.gu.mediaservice.lib.aws -case class S3Bucket(bucket: String, endpoint: String) +import java.net.URI + +case class S3Bucket(bucket: String, endpoint: String) { + def objectUrl(key: String): URI = { + val s3Endpoint = endpoint + val bucketName = bucket + val bucketUrl = if (s3Endpoint == "minio.griddev.eelpieconsulting.co.uk") { + new URI(s"http://$s3Endpoint/$bucketName/$key") + } else { + val bucketHost = s"$bucketName.$s3Endpoint" + new URI("http", bucketHost, s"/$key", null) + } + bucketUrl + } + +} From d73027d67e2332140cd82c0d12b1b2449729b102 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 14 Sep 2025 14:58:49 +0100 Subject: [PATCH 29/39] S3KeyFromURL move to S3Bucket as it know's it's own URL scheme. --- .../com/gu/mediaservice/lib/aws/S3Bucket.scala | 8 ++++++++ .../gu/mediaservice/lib/aws/S3KeyFromURL.scala | 17 ----------------- cropper/app/lib/CropStore.scala | 5 ++--- cropper/app/lib/Crops.scala | 7 +++---- media-api/app/controllers/MediaApi.scala | 12 ++++++------ media-api/app/lib/ImageResponse.scala | 7 +++---- 6 files changed, 22 insertions(+), 34 deletions(-) delete mode 100644 common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3KeyFromURL.scala diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala index 6b79669091..d68b4c07eb 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala @@ -15,4 +15,12 @@ case class S3Bucket(bucket: String, endpoint: String) { bucketUrl } + def keyFromS3URL(url: URI): String = { + if (endpoint == "minio.griddev.eelpieconsulting.co.uk") { + url.getPath.drop(bucket.length + 2) + } else { + url.getPath.drop(1) + } + } + } diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3KeyFromURL.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3KeyFromURL.scala deleted file mode 100644 index f1ecb8cbe7..0000000000 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3KeyFromURL.scala +++ /dev/null @@ -1,17 +0,0 @@ -package com.gu.mediaservice.lib.aws - -import com.gu.mediaservice.lib.logging.GridLogging - -import java.net.URI - -trait S3KeyFromURL extends GridLogging { - - def keyFromS3URL(bucket: S3Bucket, url: URI): String = { - if (bucket.endpoint == "minio.griddev.eelpieconsulting.co.uk") { - url.getPath.drop(bucket.bucket.length + 2) - } else { - url.getPath.drop(1) - } - } - -} diff --git a/cropper/app/lib/CropStore.scala b/cropper/app/lib/CropStore.scala index a3a4b32682..034790420f 100644 --- a/cropper/app/lib/CropStore.scala +++ b/cropper/app/lib/CropStore.scala @@ -4,11 +4,10 @@ import java.io.File import java.net.{URI, URL} import scala.concurrent.Future import com.gu.mediaservice.lib.S3ImageStorage -import com.gu.mediaservice.lib.aws.S3KeyFromURL import com.gu.mediaservice.lib.logging.LogMarker import com.gu.mediaservice.model._ -class CropStore(config: CropperConfig) extends S3ImageStorage(config) with CropSpecMetadata with S3KeyFromURL { +class CropStore(config: CropperConfig) extends S3ImageStorage(config) with CropSpecMetadata { import com.gu.mediaservice.lib.formatting._ def getSecureCropUri(uri: URI): Option[URL] = @@ -50,7 +49,7 @@ class CropStore(config: CropperConfig) extends S3ImageStorage(config) with CropS date = userMetadata.get("date").flatMap(parseDateTime) dimensions = Dimensions(width, height) - key = keyFromS3URL(config.imgPublishingBucket, s3Object.uri) + key = config.imgPublishingBucket.keyFromS3URL(s3Object.uri) sizing = Asset( diff --git a/cropper/app/lib/Crops.scala b/cropper/app/lib/Crops.scala index 817b0356ce..d7b3a7eda7 100644 --- a/cropper/app/lib/Crops.scala +++ b/cropper/app/lib/Crops.scala @@ -3,7 +3,7 @@ package lib import java.io.File import com.gu.mediaservice.lib.metadata.FileMetadataHelper import com.gu.mediaservice.lib.Files -import com.gu.mediaservice.lib.aws.{S3, S3Bucket, S3KeyFromURL} +import com.gu.mediaservice.lib.aws.{S3, S3Bucket} import com.gu.mediaservice.lib.imaging.{ExportResult, ImageOperations} import com.gu.mediaservice.lib.logging.{GridLogging, LogMarker, Stopwatch} import com.gu.mediaservice.model._ @@ -17,7 +17,7 @@ case object InvalidCropRequest extends Exception("Crop request invalid for image case class MasterCrop(sizing: Future[Asset], file: File, dimensions: Dimensions, aspectRatio: Float) -class Crops(config: CropperConfig, store: CropStore, imageOperations: ImageOperations, imageBucket: S3Bucket, s3: S3)(implicit ec: ExecutionContext) extends GridLogging with S3KeyFromURL { +class Crops(config: CropperConfig, store: CropStore, imageOperations: ImageOperations, imageBucket: S3Bucket, s3: S3)(implicit ec: ExecutionContext) extends GridLogging { import Files._ private val cropQuality = 75d @@ -114,8 +114,7 @@ class Crops(config: CropperConfig, store: CropStore, imageOperations: ImageOpera val hasAlpha = apiImage.fileMetadata.colourModelInformation.get("hasAlpha").flatMap(a => Try(a.toBoolean).toOption).getOrElse(true) val cropType = Crops.cropType(mimeType, colourType, hasAlpha) - val key = keyFromS3URL(imageBucket, secureFile) - + val key = imageBucket.keyFromS3URL(secureFile) val secureUrl = s3.signUrlTony(imageBucket, key) Stopwatch.async(s"making crop assets for ${apiImage.id} ${Crop.getCropId(source.bounds)}") { diff --git a/media-api/app/controllers/MediaApi.scala b/media-api/app/controllers/MediaApi.scala index bac5422571..13f5a740a4 100644 --- a/media-api/app/controllers/MediaApi.scala +++ b/media-api/app/controllers/MediaApi.scala @@ -12,7 +12,7 @@ import com.gu.mediaservice.lib.aws._ import com.gu.mediaservice.lib.aws.{ContentDisposition, Embedder, ThrallMessageSender, UpdateMessage} import com.gu.mediaservice.lib.aws.{ContentDisposition, S3, ThrallMessageSender, UpdateMessage} import com.gu.mediaservice.lib.aws.{ContentDisposition, ThrallMessageSender, UpdateMessage} -import com.gu.mediaservice.lib.aws.{ContentDisposition, S3, S3KeyFromURL, ThrallMessageSender, UpdateMessage} +import com.gu.mediaservice.lib.aws.{ContentDisposition, S3, ThrallMessageSender, UpdateMessage} import com.gu.mediaservice.lib.config.InstanceForRequest import com.gu.mediaservice.lib.events.UsageEvents import com.gu.mediaservice.lib.formatting.printDateTime @@ -52,7 +52,7 @@ class MediaApi( authorisation: Authorisation, embedder: Embedder, events: UsageEvents, -)(implicit val ec: ExecutionContext) extends BaseController with MessageSubjects with ArgoHelpers with ContentDisposition with InstanceForRequest with S3KeyFromURL { +)(implicit val ec: ExecutionContext) extends BaseController with MessageSubjects with ArgoHelpers with ContentDisposition with InstanceForRequest { private val gridClient: GridClient = GridClient(config.services)(ws) @@ -325,8 +325,8 @@ class MediaApi( val maybeResult = for { export <- source.exports.find(_.id.contains(exportId)) asset <- export.assets.find(_.dimensions.exists(_.width == width)) - key = keyFromS3URL(config.imgPublishingBucket, asset.file) - s3Object <- Try(s3Client.getObject(config.imgPublishingBucket, key)).toOption + key = config.imgPublishingBucket.keyFromS3URL(asset.file) + s3Object <- Try(s3Client.getObject(config.imgPublishingBucket, key)).toOption // TODO raw S3 client usage! file = StreamConverters.fromInputStream(() => s3Object.getObjectContent) entity = HttpEntity.Streamed(file, asset.size, asset.mimeType.map(_.name)) result = Result(ResponseHeader(OK), entity).withHeaders("Content-Disposition" -> getContentDisposition(source, export, asset, config.shortenDownloadFilename)) @@ -455,7 +455,7 @@ class MediaApi( val apiKey = request.user.accessor logger.info(logMarker, s"Download original image: $id from user: ${Authentication.getIdentity(request.user)}") mediaApiMetrics.incrementImageDownload(apiKey, mediaApiMetrics.OriginalDownloadType) - val key = keyFromS3URL(config.imageBucket, image.source.file) + val key = config.imageBucket.keyFromS3URL(image.source.file) val s3Object = s3Client.getObject(config.imageBucket, key) val file = StreamConverters.fromInputStream(() => s3Object.getObjectContent) val entity = HttpEntity.Streamed(file, image.source.size, image.source.mimeType.map(_.name)) @@ -519,7 +519,7 @@ class MediaApi( logger.info(logMarker, s"Download optimised image: $id from user: ${Authentication.getIdentity(request.user)}") mediaApiMetrics.incrementImageDownload(apiKey, mediaApiMetrics.OptimisedDownloadType) - val key = keyFromS3URL(config.imageBucket, image.optimisedPng.getOrElse(image.source).file) + val key = config.imageBucket.keyFromS3URL(image.optimisedPng.getOrElse(image.source).file) val sourceImageUri = new URI(s3Client.signUrl(config.imageBucket, key, image, imageType = image.optimisedPng match { case Some(_) => OptimisedPng diff --git a/media-api/app/lib/ImageResponse.scala b/media-api/app/lib/ImageResponse.scala index 7cd4184a62..a7ab167770 100644 --- a/media-api/app/lib/ImageResponse.scala +++ b/media-api/app/lib/ImageResponse.scala @@ -3,7 +3,6 @@ package lib import com.gu.mediaservice.lib.argo.model._ import com.gu.mediaservice.lib.auth.{Internal, Tier} import com.gu.mediaservice.lib.aws.S3 -import com.gu.mediaservice.lib.aws.S3KeyFromURL import com.gu.mediaservice.lib.collections.CollectionsManager import com.gu.mediaservice.lib.logging.{GridLogging, LogMarker} import com.gu.mediaservice.model._ @@ -21,7 +20,7 @@ import scala.annotation.tailrec import scala.util.{Failure, Try} class ImageResponse(config: MediaApiConfig, s3Client: S3, usageQuota: UsageQuota) - extends EditsResponse with GridLogging with S3KeyFromURL { + extends EditsResponse with GridLogging { implicit val usageQuotas: UsageQuota = usageQuota @@ -78,10 +77,10 @@ class ImageResponse(config: MediaApiConfig, s3Client: S3, usageQuota: UsageQuota val fileUri = image.source.file - val key = keyFromS3URL(config.imageBucket, fileUri) + val key = config.imageBucket.keyFromS3URL(fileUri) val imageUrl = s3Client.signUrl(config.imageBucket, key, image, imageType = Source) val pngUrl: Option[String] = pngFileUri - .map(uri => s3Client.signUrl(config.imageBucket, keyFromS3URL(config.imageBucket, uri), image, imageType = OptimisedPng)) + .map(uri => s3Client.signUrl(config.imageBucket, config.imageBucket.keyFromS3URL(uri), image, imageType = OptimisedPng)) def s3SignedThumbUrl = s3Client.signUrl(config.thumbnailBucket, key, image, imageType = Thumbnail) From a3aab455c98b15be790eb2d8fc422cfd0a2fd7f1 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 14 Sep 2025 15:06:09 +0100 Subject: [PATCH 30/39] Clean up; further isolating S3Client as the cloudfront concern. # Conflicts: # media-api/app/MediaApiComponents.scala # media-api/app/controllers/MediaApi.scala --- media-api/app/MediaApiComponents.scala | 8 ++++---- media-api/app/controllers/MediaApi.scala | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/media-api/app/MediaApiComponents.scala b/media-api/app/MediaApiComponents.scala index b06b6fea5c..0a57532819 100644 --- a/media-api/app/MediaApiComponents.scala +++ b/media-api/app/MediaApiComponents.scala @@ -1,4 +1,4 @@ -import com.gu.mediaservice.lib.aws.{Bedrock, Embedder, S3Vectors, ThrallMessageSender} +import com.gu.mediaservice.lib.aws._ import com.gu.mediaservice.lib.instances.InstancesClient import com.gu.mediaservice.lib.aws._ import com.gu.mediaservice.lib.management.{ElasticSearchHealthCheck, Management} @@ -18,7 +18,7 @@ class MediaApiComponents(context: Context) extends GridComponents(context, new M val messageSender = new ThrallMessageSender(config.thrallKinesisStreamConfig) val mediaApiMetrics = new MediaApiMetrics(config, actorSystem, applicationLifecycle) - val s3Client = new S3(config) + val s3 = new S3(config) val usageQuota = new UsageQuota(config, actorSystem.scheduler) usageQuota.quotaStore.update() @@ -28,12 +28,12 @@ class MediaApiComponents(context: Context) extends GridComponents(context, new M val elasticSearch = new ElasticSearch(config, mediaApiMetrics, config.esConfig, () => usageQuota.usageStore.overQuotaAgencies, actorSystem.scheduler, new InstancesClient(config, wsClient)) // TODO needs to move somewhere more instance aware elasticSearch.ensureIndexExistsAndAliasAssigned() - val imageResponse = new ImageResponse(config, s3Client, usageQuota) + val imageResponse = new ImageResponse(config, s3, usageQuota) val softDeletedMetadataTable = new SoftDeletedMetadataTable(config) val embedder = new Embedder(new Bedrock(config), new SimpleSqsMessageConsumer(config.queueUrl, config)) - val mediaApi = new MediaApi(auth, messageSender, softDeletedMetadataTable, elasticSearch, imageResponse, config, controllerComponents, s3Client, mediaApiMetrics, wsClient, authorisation, embedder, usageEvents) + val mediaApi = new MediaApi(auth, messageSender, softDeletedMetadataTable, elasticSearch, imageResponse, config, controllerComponents, s3, mediaApiMetrics, wsClient, authorisation, embedder, usageEvents) val suggestionController = new SuggestionController(auth, elasticSearch, controllerComponents) val aggController = new AggregationController(auth, elasticSearch, controllerComponents) val usageController = new UsageController(auth, config, elasticSearch, usageQuota, controllerComponents) diff --git a/media-api/app/controllers/MediaApi.scala b/media-api/app/controllers/MediaApi.scala index 13f5a740a4..8509c07c79 100644 --- a/media-api/app/controllers/MediaApi.scala +++ b/media-api/app/controllers/MediaApi.scala @@ -46,7 +46,7 @@ class MediaApi( imageResponse: ImageResponse, config: MediaApiConfig, override val controllerComponents: ControllerComponents, - s3Client: S3, + s3: S3, mediaApiMetrics: MediaApiMetrics, ws: WSClient, authorisation: Authorisation, @@ -326,7 +326,7 @@ class MediaApi( export <- source.exports.find(_.id.contains(exportId)) asset <- export.assets.find(_.dimensions.exists(_.width == width)) key = config.imgPublishingBucket.keyFromS3URL(asset.file) - s3Object <- Try(s3Client.getObject(config.imgPublishingBucket, key)).toOption // TODO raw S3 client usage! + s3Object <- Try(s3.getObject(config.imgPublishingBucket, key)).toOption file = StreamConverters.fromInputStream(() => s3Object.getObjectContent) entity = HttpEntity.Streamed(file, asset.size, asset.mimeType.map(_.name)) result = Result(ResponseHeader(OK), entity).withHeaders("Content-Disposition" -> getContentDisposition(source, export, asset, config.shortenDownloadFilename)) @@ -456,7 +456,7 @@ class MediaApi( logger.info(logMarker, s"Download original image: $id from user: ${Authentication.getIdentity(request.user)}") mediaApiMetrics.incrementImageDownload(apiKey, mediaApiMetrics.OriginalDownloadType) val key = config.imageBucket.keyFromS3URL(image.source.file) - val s3Object = s3Client.getObject(config.imageBucket, key) + val s3Object = s3.getObject(config.imageBucket, key) val file = StreamConverters.fromInputStream(() => s3Object.getObjectContent) val entity = HttpEntity.Streamed(file, image.source.size, image.source.mimeType.map(_.name)) @@ -521,7 +521,7 @@ class MediaApi( val key = config.imageBucket.keyFromS3URL(image.optimisedPng.getOrElse(image.source).file) val sourceImageUri = - new URI(s3Client.signUrl(config.imageBucket, key, image, imageType = image.optimisedPng match { + new URI(s3.signUrl(config.imageBucket, key, image, imageType = image.optimisedPng match { case Some(_) => OptimisedPng case _ => Source })) From 24e0936f136fb8bc5ad61e53f2eb5e3ce64a4eed Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 14 Sep 2025 15:09:01 +0100 Subject: [PATCH 31/39] Refactor; extracting the is path style URLs bucket decision. --- .../main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala index d68b4c07eb..26024c6711 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala @@ -6,7 +6,7 @@ case class S3Bucket(bucket: String, endpoint: String) { def objectUrl(key: String): URI = { val s3Endpoint = endpoint val bucketName = bucket - val bucketUrl = if (s3Endpoint == "minio.griddev.eelpieconsulting.co.uk") { + val bucketUrl = if (isPathStyleURLs(s3Endpoint)) { new URI(s"http://$s3Endpoint/$bucketName/$key") } else { val bucketHost = s"$bucketName.$s3Endpoint" @@ -16,11 +16,15 @@ case class S3Bucket(bucket: String, endpoint: String) { } def keyFromS3URL(url: URI): String = { - if (endpoint == "minio.griddev.eelpieconsulting.co.uk") { + if (isPathStyleURLs(endpoint)) { url.getPath.drop(bucket.length + 2) } else { url.getPath.drop(1) } } + private def isPathStyleURLs(s3Endpoint: String) = { + s3Endpoint == "minio.griddev.eelpieconsulting.co.uk" + } + } From cd04a161d22710eb668b3c3e47777e67df22e0f2 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 14 Sep 2025 15:17:07 +0100 Subject: [PATCH 32/39] S3 path based URLs decision moves to bucket property and config. --- .../scala/com/gu/mediaservice/lib/aws/S3Bucket.scala | 10 +++------- .../com/gu/mediaservice/lib/config/CommonConfig.scala | 10 +++++----- cropper/app/lib/CropperConfig.scala | 3 ++- cropper/test/lib/CropsTest.scala | 2 +- image-loader/app/lib/ImageLoaderConfig.scala | 2 +- image-loader/test/scala/model/ImageUploadTest.scala | 4 ++-- image-loader/test/scala/model/ProjectorTest.scala | 2 +- media-api/app/lib/MediaApiConfig.scala | 8 ++++---- .../auth/provider/ApiKeyAuthenticationProvider.scala | 2 +- .../lib/auth/ApiKeyAuthenticationProviderTest.scala | 2 +- thrall/app/lib/ThrallConfig.scala | 2 +- 11 files changed, 22 insertions(+), 25 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala index 26024c6711..f9ac5b9cc5 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala @@ -2,11 +2,11 @@ package com.gu.mediaservice.lib.aws import java.net.URI -case class S3Bucket(bucket: String, endpoint: String) { +case class S3Bucket(bucket: String, endpoint: String, usesPathStyleURLs: Boolean) { def objectUrl(key: String): URI = { val s3Endpoint = endpoint val bucketName = bucket - val bucketUrl = if (isPathStyleURLs(s3Endpoint)) { + val bucketUrl = if (usesPathStyleURLs) { new URI(s"http://$s3Endpoint/$bucketName/$key") } else { val bucketHost = s"$bucketName.$s3Endpoint" @@ -16,15 +16,11 @@ case class S3Bucket(bucket: String, endpoint: String) { } def keyFromS3URL(url: URI): String = { - if (isPathStyleURLs(endpoint)) { + if (usesPathStyleURLs) { url.getPath.drop(bucket.length + 2) } else { url.getPath.drop(1) } } - private def isPathStyleURLs(s3Endpoint: String) = { - s3Endpoint == "minio.griddev.eelpieconsulting.co.uk" - } - } diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala index df7b9d3a10..e5474312b3 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala @@ -57,16 +57,16 @@ abstract class CommonConfig(resources: GridConfigResources) extends AwsClientV1B ingestBucket <- stringOpt("s3.ingest.bucket.name") ingestBucketEndpoint <- stringOpt("s3.ingest.bucket.endpoint") } yield { - S3Bucket(ingestBucket, ingestBucketEndpoint) + S3Bucket(ingestBucket, ingestBucketEndpoint, usesPathStyleURLs = booleanOpt("s3.ingest.bucket.pathStyleURLs").getOrElse(false)) } val maybeFailBucket: Option[S3Bucket] = for { failBucket <- stringOpt("s3.fail.bucket.name") failBucketEndpoint <- stringOpt("s3.fail.bucket.endpoint") } yield { - S3Bucket(failBucket, failBucketEndpoint) + S3Bucket(failBucket, failBucketEndpoint, usesPathStyleURLs = booleanOpt("s3.fail.bucket.pathStyleURLs").getOrElse(false)) } - val maybeQuarantineBucket: Option[S3Bucket] = stringOpt("s3.quarantine.bucket").map(S3Bucket(_, S3.AmazonAwsS3Endpoint)) + val maybeQuarantineBucket: Option[S3Bucket] = stringOpt("s3.quarantine.bucket").map(S3Bucket(_, S3.AmazonAwsS3Endpoint, usesPathStyleURLs = booleanOpt("s3.fail.bucket.pathStyleURLs").getOrElse(false))) val maybeBucketForUIUploads: Option[S3Bucket] = maybeQuarantineBucket orElse maybeIngestBucket @@ -83,8 +83,8 @@ abstract class CommonConfig(resources: GridConfigResources) extends AwsClientV1B val services = new SingleHostServices(domainRoot) - val imageBucket: S3Bucket = S3Bucket(string("s3.image.bucket.name"), string("s3.image.bucket.endpoint")) - val thumbnailBucket: S3Bucket = S3Bucket(string("s3.thumb.bucket.name"), string("s3.thumb.bucket.endpoint")) + val imageBucket: S3Bucket = S3Bucket(string("s3.image.bucket.name"), string("s3.image.bucket.endpoint"), usesPathStyleURLs = booleanOpt("s3.image.bucket.pathStyleURLs").getOrElse(false)) + val thumbnailBucket: S3Bucket = S3Bucket(string("s3.thumb.bucket.name"), string("s3.thumb.bucket.endpoint"), usesPathStyleURLs = booleanOpt("s3.thumb.bucket.pathStyleURLs").getOrElse(false)) val googleS3AccessKey = stringOpt("s3.accessKey") val googleS3SecretKey = stringOpt("s3.secretKey") diff --git a/cropper/app/lib/CropperConfig.scala b/cropper/app/lib/CropperConfig.scala index 395122932c..979824eebb 100644 --- a/cropper/app/lib/CropperConfig.scala +++ b/cropper/app/lib/CropperConfig.scala @@ -10,7 +10,8 @@ import java.io.File class CropperConfig(resources: GridConfigResources) extends CommonConfig(resources) { val imgPublishingBucket: S3Bucket = S3Bucket( string("publishing.image.bucket"), - S3.AmazonAwsS3Endpoint + S3.AmazonAwsS3Endpoint, + usesPathStyleURLs = false ) val canDownloadCrop: Boolean = boolean("canDownloadCrop") diff --git a/cropper/test/lib/CropsTest.scala b/cropper/test/lib/CropsTest.scala index 52cb6f82b7..6a0a64e49c 100644 --- a/cropper/test/lib/CropsTest.scala +++ b/cropper/test/lib/CropsTest.scala @@ -58,7 +58,7 @@ class CropsTest extends AnyFunSpec with Matchers with MockitoSugar { private val source: SourceImage = SourceImage("test", mock[Asset], valid = true, mock[ImageMetadata], mock[FileMetadata]) private val bounds: Bounds = Bounds(10, 20, 30, 40) private val outputWidth = 1234 - private val imageBucket = S3Bucket("crops-bucket", S3.AmazonAwsS3Endpoint) + private val imageBucket = S3Bucket("crops-bucket", S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false) private val s3 = new S3(config) it("should should construct a correct address for a master jpg") { diff --git a/image-loader/app/lib/ImageLoaderConfig.scala b/image-loader/app/lib/ImageLoaderConfig.scala index ceb923387f..a214d82845 100644 --- a/image-loader/app/lib/ImageLoaderConfig.scala +++ b/image-loader/app/lib/ImageLoaderConfig.scala @@ -16,7 +16,7 @@ class ImageLoaderConfig(resources: GridConfigResources) extends CommonConfig(res val maybeImageReplicaBucket: Option[String] = stringOpt("s3.image.replicaBucket") val quarantineBucket: Option[S3Bucket] = stringOpt("s3.quarantine.bucket").map { bucket => - S3Bucket(bucket, S3.AmazonAwsS3Endpoint) + S3Bucket(bucket, S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false) } val uploadToQuarantineEnabled: Boolean = boolean("upload.quarantine.enabled") diff --git a/image-loader/test/scala/model/ImageUploadTest.scala b/image-loader/test/scala/model/ImageUploadTest.scala index 6c9bd823c2..adcb572978 100644 --- a/image-loader/test/scala/model/ImageUploadTest.scala +++ b/image-loader/test/scala/model/ImageUploadTest.scala @@ -36,7 +36,7 @@ class ImageUploadTest extends AsyncFunSuite with Matchers with MockitoSugar { private implicit val logMarker: MockLogMarker = new MockLogMarker() // For mime type info, see https://github.com/guardian/grid/pull/2568 val tempDir = new File("/tmp") - val mockConfig: ImageUploadOpsCfg = ImageUploadOpsCfg(tempDir, 256, 85d, List(Tiff), S3Bucket("img-bucket", S3.AmazonAwsS3Endpoint), S3Bucket("thumb-bucket", S3.AmazonAwsS3Endpoint)) + val mockConfig: ImageUploadOpsCfg = ImageUploadOpsCfg(tempDir, 256, 85d, List(Tiff), S3Bucket("img-bucket", S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false), S3Bucket("thumb-bucket", S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false)) /** * @todo: I flailed about until I found a path that worked, but @@ -57,7 +57,7 @@ class ImageUploadTest extends AsyncFunSuite with Matchers with MockitoSugar { def mockStore = (a: StorableImage) => Future.successful( - S3Object(S3Bucket("madeupname", S3.AmazonAwsS3Endpoint), "madeupkey", a.file, Some(a.mimeType), None, a.meta, None) + S3Object(S3Bucket("madeupname", S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false), "madeupkey", a.file, Some(a.mimeType), None, a.meta, None) ) def storeOrProjectOriginalFile: StorableOriginalImage => Future[S3Object] = mockStore diff --git a/image-loader/test/scala/model/ProjectorTest.scala b/image-loader/test/scala/model/ProjectorTest.scala index f145a237be..89e77c1b98 100644 --- a/image-loader/test/scala/model/ProjectorTest.scala +++ b/image-loader/test/scala/model/ProjectorTest.scala @@ -42,7 +42,7 @@ class ProjectorTest extends AnyFreeSpec with Matchers with ScalaFutures with Moc private val imageOperations = new ImageOperations(ctxPath) - private val config = ImageUploadOpsCfg(new File("/tmp"), 256, 85d, Nil, S3Bucket("img-bucket", S3.AmazonAwsS3Endpoint), S3Bucket("thumb-bucket", S3.AmazonAwsS3Endpoint)) + private val config = ImageUploadOpsCfg(new File("/tmp"), 256, 85d, Nil, S3Bucket("img-bucket", S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false), S3Bucket("thumb-bucket", S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false)) private val maybeEmbedder = None diff --git a/media-api/app/lib/MediaApiConfig.scala b/media-api/app/lib/MediaApiConfig.scala index 44a911cd31..79c312c938 100644 --- a/media-api/app/lib/MediaApiConfig.scala +++ b/media-api/app/lib/MediaApiConfig.scala @@ -20,17 +20,17 @@ case class StoreConfig( ) class MediaApiConfig(resources: GridConfigResources) extends CommonConfigWithElastic(resources) { - val configBucket: S3Bucket = S3Bucket(string("s3.config.bucket"), S3.AmazonAwsS3Endpoint) - val usageMailBucket: S3Bucket = S3Bucket(string("s3.usagemail.bucket"), S3.AmazonAwsS3Endpoint) + val configBucket: S3Bucket = S3Bucket(string("s3.config.bucket"), S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false) + val usageMailBucket: S3Bucket = S3Bucket(string("s3.usagemail.bucket"), S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false) val quotaStoreKey: String = string("quota.store.key") val quotaStoreConfig: StoreConfig = StoreConfig(configBucket, quotaStoreKey) //Lazy allows this to be empty and not break things unless used somewhere - lazy val imgPublishingBucket: S3Bucket = S3Bucket(string("publishing.image.bucket"), S3.AmazonAwsS3Endpoint) + lazy val imgPublishingBucket: S3Bucket = S3Bucket(string("publishing.image.bucket"), S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false) val cloudFrontDomainThumbBucket: Option[String] = stringOpt("cloudfront.domain.thumbbucket") - val cloudFrontPrivateKeyBucket: Option[S3Bucket] = stringOpt("cloudfront.private-key.bucket").map(S3Bucket(_, S3.AmazonAwsS3Endpoint)) + val cloudFrontPrivateKeyBucket: Option[S3Bucket] = stringOpt("cloudfront.private-key.bucket").map(S3Bucket(_, S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false)) val cloudFrontPrivateKeyBucketKey: Option[String] = stringOpt("cloudfront.private-key.key") val cloudFrontKeyPairId: Option[String] = stringOpt("cloudfront.keypair.id") diff --git a/rest-lib/src/main/scala/com/gu/mediaservice/lib/auth/provider/ApiKeyAuthenticationProvider.scala b/rest-lib/src/main/scala/com/gu/mediaservice/lib/auth/provider/ApiKeyAuthenticationProvider.scala index e5ada4c33f..87079a4fac 100644 --- a/rest-lib/src/main/scala/com/gu/mediaservice/lib/auth/provider/ApiKeyAuthenticationProvider.scala +++ b/rest-lib/src/main/scala/com/gu/mediaservice/lib/auth/provider/ApiKeyAuthenticationProvider.scala @@ -26,7 +26,7 @@ class ApiKeyAuthenticationProvider(configuration: Configuration, resources: Auth var keyStorePlaceholder: Option[KeyStore] = _ override def initialise(): Unit = { - val authKeyStoreBucket = S3Bucket(configuration.get[String]("authKeyStoreBucket"), S3.AmazonAwsS3Endpoint) + val authKeyStoreBucket = S3Bucket(configuration.get[String]("authKeyStoreBucket"), S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false) val store = new KeyStore(authKeyStoreBucket, resources.commonConfig) store.scheduleUpdates(resources.actorSystem.scheduler) keyStorePlaceholder = Some(store) diff --git a/rest-lib/src/test/scala/com/gu/mediaservice/lib/auth/ApiKeyAuthenticationProviderTest.scala b/rest-lib/src/test/scala/com/gu/mediaservice/lib/auth/ApiKeyAuthenticationProviderTest.scala index 220302deee..76c2bfd1b4 100644 --- a/rest-lib/src/test/scala/com/gu/mediaservice/lib/auth/ApiKeyAuthenticationProviderTest.scala +++ b/rest-lib/src/test/scala/com/gu/mediaservice/lib/auth/ApiKeyAuthenticationProviderTest.scala @@ -44,7 +44,7 @@ class ApiKeyAuthenticationProviderTest extends AsyncFreeSpec with Matchers with Future.successful(()) } - override def keyStore: KeyStore = new KeyStore(S3Bucket("not-used", S3.AmazonAwsS3Endpoint), resources.commonConfig) { + override def keyStore: KeyStore = new KeyStore(S3Bucket("not-used", S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false), resources.commonConfig) { override def lookupIdentity(key: String)(implicit instance: Instance): Option[ApiAccessor] = { key match { case "key-chuckle" => Some(ApiAccessor("brothers", Internal)) diff --git a/thrall/app/lib/ThrallConfig.scala b/thrall/app/lib/ThrallConfig.scala index 1a08923f9f..48ab0dd971 100644 --- a/thrall/app/lib/ThrallConfig.scala +++ b/thrall/app/lib/ThrallConfig.scala @@ -56,7 +56,7 @@ object KinesisReceiverConfig { } class ThrallConfig(resources: GridConfigResources) extends CommonConfigWithElastic(resources) { - val maybeReaperBucket: Option[S3Bucket] = stringOpt("s3.reaper.bucket").map(S3Bucket(_, S3.AmazonAwsS3Endpoint)) + val maybeReaperBucket: Option[S3Bucket] = stringOpt("s3.reaper.bucket").map(S3Bucket(_, S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false)) val maybeReaperCountPerRun: Option[Int] = intOpt("reaper.countPerRun") val metadataTopicArn: String = string("indexed.image.sns.topic.arn") From 1a13d05a06bf0556380cc65bdce98e887260ff40 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sat, 13 Dec 2025 10:48:57 +0000 Subject: [PATCH 33/39] Extract bucket base URL function. Upload bucket connectSource is supplied by the bucket object; knows about path style bucket URLs. Preserve the objectUrl is http contract. Not sure if this is important or not. --- .../gu/mediaservice/lib/aws/S3Bucket.scala | 19 ++++++++++--------- kahuna/app/lib/KahunaConfig.scala | 6 +++--- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala index f9ac5b9cc5..bcc3c58ae5 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala @@ -4,15 +4,8 @@ import java.net.URI case class S3Bucket(bucket: String, endpoint: String, usesPathStyleURLs: Boolean) { def objectUrl(key: String): URI = { - val s3Endpoint = endpoint - val bucketName = bucket - val bucketUrl = if (usesPathStyleURLs) { - new URI(s"http://$s3Endpoint/$bucketName/$key") - } else { - val bucketHost = s"$bucketName.$s3Endpoint" - new URI("http", bucketHost, s"/$key", null) - } - bucketUrl + val bucketBaseURL = bucketURL() + new URI("http", bucketBaseURL.getHost, bucketBaseURL.getPath + key, null) } def keyFromS3URL(url: URI): String = { @@ -23,4 +16,12 @@ case class S3Bucket(bucket: String, endpoint: String, usesPathStyleURLs: Boolean } } + def bucketURL(): URI = { + if (usesPathStyleURLs) { + new URI("https", endpoint, s"/$bucket/", null) + } else { + new URI("https", s"$bucket.$endpoint", "/", null) + } + } + } diff --git a/kahuna/app/lib/KahunaConfig.scala b/kahuna/app/lib/KahunaConfig.scala index 721b79e199..8be882a61a 100644 --- a/kahuna/app/lib/KahunaConfig.scala +++ b/kahuna/app/lib/KahunaConfig.scala @@ -2,6 +2,7 @@ package lib import com.gu.mediaservice.lib.auth.Permissions.Pinboard import com.gu.mediaservice.lib.auth.SimplePermission +import com.gu.mediaservice.lib.aws.S3Bucket import com.gu.mediaservice.lib.config.{CommonConfig, GridConfigResources} import com.gu.mediaservice.model.Instance import play.api.libs.json._ @@ -49,9 +50,8 @@ class KahunaConfig(resources: GridConfigResources) extends CommonConfig(resource val showSendToPhotoSales: Option[Boolean] = booleanOpt("showSendToPhotoSales") val frameAncestors: Set[String] = getStringSet("security.frameAncestors") - val connectSources: Set[String] = getStringSet("security.connectSources") ++ maybeBucketForUIUploads.map { bucket => - if (isDev) "https://localstack.media.local.dev-gutools.co.uk" - else s"https://$bucket.s3.$awsRegion.amazonaws.com" // TODO push to bucket end point + val connectSources: Set[String] = getStringSet("security.connectSources") ++ maybeIngestBucket.map { ingestBucket => + ingestBucket.bucketURL().toURL.toExternalForm } ++ telemetryUri val fontSources: Set[String] = getStringSet("security.fontSources") val imageSources: Set[String] = getStringSet("security.imageSources") From ae581bc2353da1b9a9fa6d7e9fc9634ef81c284d Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 25 Jan 2026 12:11:28 +0000 Subject: [PATCH 34/39] Crops bucket can be on non AWS bucket. --- cropper/app/lib/CropperConfig.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cropper/app/lib/CropperConfig.scala b/cropper/app/lib/CropperConfig.scala index 979824eebb..4a4739d8ce 100644 --- a/cropper/app/lib/CropperConfig.scala +++ b/cropper/app/lib/CropperConfig.scala @@ -9,9 +9,9 @@ import java.io.File class CropperConfig(resources: GridConfigResources) extends CommonConfig(resources) { val imgPublishingBucket: S3Bucket = S3Bucket( - string("publishing.image.bucket"), - S3.AmazonAwsS3Endpoint, - usesPathStyleURLs = false + string("publishing.image.bucket.name"), + string("publishing.image.bucket.endpoint"), + boolean("publishing.image.bucket.pathStyleURLs") ) val canDownloadCrop: Boolean = boolean("canDownloadCrop") From 46b4bfa7ddace08b69bcc02b7b8d4403e5cbdc86 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sat, 23 May 2026 19:11:08 +0100 Subject: [PATCH 35/39] S3 Client moves onto the S3 Bucket.  Conflicts:  common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala  common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala  image-loader/test/scala/model/ImageUploadTest.scala --- .../com/gu/mediaservice/lib/aws/S3.scala | 62 +++++++------------ .../gu/mediaservice/lib/aws/S3Bucket.scala | 4 +- .../lib/config/CommonConfig.scala | 38 +++++++++--- cropper/app/lib/CropperConfig.scala | 3 +- cropper/test/lib/CropsTest.scala | 4 +- image-loader/app/lib/ImageLoaderConfig.scala | 3 +- .../test/scala/model/ImageUploadTest.scala | 17 ++--- .../test/scala/model/ProjectorTest.scala | 8 ++- media-api/app/lib/MediaApiConfig.scala | 11 ++-- .../ApiKeyAuthenticationProvider.scala | 9 +-- .../ApiKeyAuthenticationProviderTest.scala | 4 +- thrall/app/lib/ThrallConfig.scala | 2 +- 12 files changed, 89 insertions(+), 76 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala index c6c44037b9..a4266ff887 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala @@ -64,73 +64,57 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with val AmazonAwsS3Endpoint: String = S3.AmazonAwsS3Endpoint - private lazy val amazonS3: AmazonS3 = S3Ops.buildS3Client(config) - private val googleS3: Option[AmazonS3] = S3Ops.buildGoogleS3Client(config) - private val localS3: Option[AmazonS3] = S3Ops.buildLocalS3Client(config) - - private def clientFor(bucket: S3Bucket): AmazonS3 = { - (bucket.endpoint match { - case "storage.googleapis.com" => - googleS3 - case "minio.griddev.eelpieconsulting.co.uk" => - localS3 - case _ => - Some(amazonS3) - }).getOrElse { - amazonS3 - } - } - def signUrl(bucket: S3Bucket, key: String, image: Image, expiration: DateTime = cachableExpiration(), imageType: ImageFileType = Source): String = { val contentDisposition = getContentDisposition(image, imageType, config.shortenDownloadFilename) val headers = new ResponseHeaderOverrides().withContentDisposition(contentDisposition) val request = new GeneratePresignedUrlRequest(bucket.bucket, key).withExpiration(expiration.toDate).withResponseHeaders(headers) - clientFor(bucket).generatePresignedUrl(request).toExternalForm + bucket.client.generatePresignedUrl(request).toExternalForm } def signUrlTony(bucket: S3Bucket, key: String, expiration: DateTime = cachableExpiration()): URL = { val request = new GeneratePresignedUrlRequest(bucket.bucket, key).withExpiration(expiration.toDate) - clientFor(bucket).generatePresignedUrl(request) + bucket.client.generatePresignedUrl(request) } def copyObject(sourceBucket: S3Bucket, destinationBucket: S3Bucket, key: String): CopyObjectResult = { - clientFor(sourceBucket).copyObject(sourceBucket.bucket, key, destinationBucket.bucket, key) + // TODO check that source and destination share the same client + sourceBucket.client.copyObject(sourceBucket.bucket, key, destinationBucket.bucket, key) } def generatePresignedRequest(request: GeneratePresignedUrlRequest, bucket: S3Bucket): URL = { - clientFor(bucket).generatePresignedUrl(request) + bucket.client.generatePresignedUrl(request) } def deleteObject(bucket: S3Bucket, key: String): Unit = { - clientFor(bucket).deleteObject(bucket.bucket, key) + bucket.client.deleteObject(bucket.bucket, key) } def deleteObjects(bucket: S3Bucket, keys: Seq[String]): DeleteObjectsResult = { - clientFor(bucket).deleteObjects( + bucket.client.deleteObjects( new DeleteObjectsRequest(bucket.bucket).withKeys(keys: _*) ) } def deleteVersion(bucket: S3Bucket, id: String, objectVersion: String): Unit = { - clientFor(bucket).deleteVersion(bucket.bucket, id, objectVersion) + bucket.client.deleteVersion(bucket.bucket, id, objectVersion) } def doesObjectExist(bucket: S3Bucket, key: String) = { - clientFor(bucket).doesObjectExist(bucket.bucket, key) + bucket.client.doesObjectExist(bucket.bucket, key) } def getObject(bucket: S3Bucket, key: String): model.S3Object = { - clientFor(bucket).getObject(new GetObjectRequest(bucket.bucket, key)) + bucket.client.getObject(new GetObjectRequest(bucket.bucket, key)) } def getObject(bucket: S3Bucket, obj: S3ObjectSummary): model.S3Object = { - clientFor(bucket).getObject(bucket.bucket, obj.getKey) + bucket.client.getObject(bucket.bucket, obj.getKey) } def getObjectAsString(bucket: S3Bucket, key: String): Option[String] = { - val content = clientFor(bucket).getObject(new GetObjectRequest(bucket.bucket, key)) + val content = bucket.client.getObject(new GetObjectRequest(bucket.bucket, key)) val stream = content.getObjectContent try { Some(IOUtils.toString(stream).trim) @@ -145,23 +129,23 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with } def getObjectMetadata(bucket: S3Bucket, id: String): ObjectMetadata = { - clientFor(bucket).getObjectMetadata(bucket.bucket, id) + bucket.client.getObjectMetadata(bucket.bucket, id) } def listObjects(bucket: S3Bucket): ObjectListing = { - clientFor(bucket).listObjects(bucket.bucket) + bucket.client.listObjects(bucket.bucket) } def listObjects(bucket: S3Bucket, prefix: String): ObjectListing = { - clientFor(bucket).listObjects(bucket.bucket, prefix) + bucket.client.listObjects(bucket.bucket, prefix) } def listObjectKeys(bucket: S3Bucket): Seq[String] = { - clientFor(bucket).listObjects(bucket.bucket).getObjectSummaries.asScala.map(_.getKey).toSeq + bucket.client.listObjects(bucket.bucket).getObjectSummaries.asScala.map(_.getKey).toSeq } def putObject(bucket: S3Bucket, key: String, content: String): Unit = { - clientFor(bucket).putObject(bucket.bucket, key, content) + bucket.client.putObject(bucket.bucket, key, content) } def store(bucket: S3Bucket, id: Key, file: File, mimeType: Option[MimeType], meta: UserMetadata = Map.empty, cacheControl: Option[String] = None) @@ -181,7 +165,7 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with val req = new PutObjectRequest(bucket.bucket, id, file).withMetadata(metadata) Stopwatch(s"S3 client.putObject ($req)"){ - val client = clientFor(bucket) + val client = bucket.client client.putObject(req) // once we've completed the PUT read back to ensure that we are returning reality val metadata = client.getObjectMetadata(bucket.bucket, id) @@ -192,7 +176,7 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with def storeIfNotPresent(bucket: S3Bucket, id: Key, file: File, mimeType: Option[MimeType], meta: UserMetadata = Map.empty, cacheControl: Option[String] = None) (implicit ex: ExecutionContext, logMarker: LogMarker): Future[S3Object] = { Future{ - Some(clientFor(bucket).getObjectMetadata(bucket.bucket, id)) + Some(bucket.client.getObjectMetadata(bucket.bucket, id)) }.recover { // translate this exception into the object not existing case as3e:AmazonS3Exception if as3e.getStatusCode == 404 => None @@ -209,7 +193,7 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with (implicit ex: ExecutionContext): Future[List[S3Object]] = Future { val req = new ListObjectsRequest().withBucketName(bucket.bucket).withPrefix(s"$prefixDir/") - val listing = clientFor(bucket).listObjects(req) + val listing = bucket.client.listObjects(req) val summaries = listing.getObjectSummaries.asScala summaries.map(summary => (summary.getKey, summary)).foldLeft(List[S3Object]()) { case (memo: List[S3Object], (key: String, summary: S3ObjectSummary)) => @@ -218,16 +202,16 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with } def getMetadata(bucket: S3Bucket, key: Key): S3Metadata = { - val meta = clientFor(bucket).getObjectMetadata(bucket.bucket, key) + val meta = bucket.client.getObjectMetadata(bucket.bucket, key) S3Metadata(meta) } def getUserMetadata(bucket: S3Bucket, key: Key): Map[String, String] = - clientFor(bucket).getObjectMetadata(bucket.bucket, key).getUserMetadata.asScala.toMap + bucket.client.getObjectMetadata(bucket.bucket, key).getUserMetadata.asScala.toMap def syncFindKey(bucket: S3Bucket, prefixName: String): Option[Key] = { val req = new ListObjectsRequest().withBucketName(bucket.bucket).withPrefix(s"$prefixName-") - val listing = clientFor(bucket).listObjects(req) + val listing = bucket.client.listObjects(req) val summaries = listing.getObjectSummaries.asScala summaries.headOption.map(_.getKey) } diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala index bcc3c58ae5..9f21737558 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3Bucket.scala @@ -1,8 +1,10 @@ package com.gu.mediaservice.lib.aws +import com.amazonaws.services.s3.AmazonS3 + import java.net.URI -case class S3Bucket(bucket: String, endpoint: String, usesPathStyleURLs: Boolean) { +case class S3Bucket(bucket: String, endpoint: String, usesPathStyleURLs: Boolean, client: AmazonS3) { def objectUrl(key: String): URI = { val bucketBaseURL = bucketURL() new URI("http", bucketBaseURL.getHost, bucketBaseURL.getPath + key, null) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala index e5474312b3..48853eaf2b 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala @@ -1,6 +1,7 @@ package com.gu.mediaservice.lib.config -import com.gu.mediaservice.lib.aws.{AwsClientV1BuilderUtils, AwsClientV2BuilderUtils, KinesisSenderConfig, S3, S3Ops, S3Bucket} +import com.amazonaws.services.s3.AmazonS3 +import com.gu.mediaservice.lib.aws._ import com.gu.mediaservice.model.UsageRightsSpec import com.typesafe.config.Config import com.typesafe.scalalogging.StrictLogging @@ -52,21 +53,41 @@ abstract class CommonConfig(resources: GridConfigResources) extends AwsClientV1B lazy val softDeletedMetadataTable: String = string("dynamo.table.softDelete.metadata") + val googleS3AccessKey: Option[String] = stringOpt("s3.accessKey") + val googleS3SecretKey: Option[String] = stringOpt("s3.secretKey") + + private val amazonS3: AmazonS3 = S3Ops.buildS3Client(this) + private val googleS3: Option[AmazonS3] = S3Ops.buildGoogleS3Client(this) + private val localS3: Option[AmazonS3] = S3Ops.buildLocalS3Client(this) + + def clientFor(bucketEndpoint: String): AmazonS3 = { + (bucketEndpoint match { + case "storage.googleapis.com" => + googleS3 + case "minio.griddev.eelpieconsulting.co.uk" => + localS3 + case _ => + Some(amazonS3) + }).getOrElse { + amazonS3 + } + } + val maybeIngestSqsQueueUrl: Option[String] = stringOpt("sqs.ingest.queue.url") val maybeIngestBucket: Option[S3Bucket] = for { ingestBucket <- stringOpt("s3.ingest.bucket.name") ingestBucketEndpoint <- stringOpt("s3.ingest.bucket.endpoint") } yield { - S3Bucket(ingestBucket, ingestBucketEndpoint, usesPathStyleURLs = booleanOpt("s3.ingest.bucket.pathStyleURLs").getOrElse(false)) + S3Bucket(ingestBucket, ingestBucketEndpoint, usesPathStyleURLs = booleanOpt("s3.ingest.bucket.pathStyleURLs").getOrElse(false), clientFor(ingestBucketEndpoint)) } val maybeFailBucket: Option[S3Bucket] = for { failBucket <- stringOpt("s3.fail.bucket.name") failBucketEndpoint <- stringOpt("s3.fail.bucket.endpoint") } yield { - S3Bucket(failBucket, failBucketEndpoint, usesPathStyleURLs = booleanOpt("s3.fail.bucket.pathStyleURLs").getOrElse(false)) + S3Bucket(failBucket, failBucketEndpoint, usesPathStyleURLs = booleanOpt("s3.fail.bucket.pathStyleURLs").getOrElse(false), clientFor(failBucketEndpoint)) } - val maybeQuarantineBucket: Option[S3Bucket] = stringOpt("s3.quarantine.bucket").map(S3Bucket(_, S3.AmazonAwsS3Endpoint, usesPathStyleURLs = booleanOpt("s3.fail.bucket.pathStyleURLs").getOrElse(false))) + val maybeQuarantineBucket: Option[S3Bucket] = stringOpt("s3.quarantine.bucket.name").map(S3Bucket(_, S3.AmazonAwsS3Endpoint, booleanOpt("s3.quarantine.bucket.pathStyleURLs").getOrElse(false), clientFor(S3.AmazonAwsS3Endpoint))) val maybeBucketForUIUploads: Option[S3Bucket] = maybeQuarantineBucket orElse maybeIngestBucket @@ -83,11 +104,10 @@ abstract class CommonConfig(resources: GridConfigResources) extends AwsClientV1B val services = new SingleHostServices(domainRoot) - val imageBucket: S3Bucket = S3Bucket(string("s3.image.bucket.name"), string("s3.image.bucket.endpoint"), usesPathStyleURLs = booleanOpt("s3.image.bucket.pathStyleURLs").getOrElse(false)) - val thumbnailBucket: S3Bucket = S3Bucket(string("s3.thumb.bucket.name"), string("s3.thumb.bucket.endpoint"), usesPathStyleURLs = booleanOpt("s3.thumb.bucket.pathStyleURLs").getOrElse(false)) - - val googleS3AccessKey = stringOpt("s3.accessKey") - val googleS3SecretKey = stringOpt("s3.secretKey") + private val imageBucketEndpoint = string("s3.image.bucket.endpoint") + val imageBucket: S3Bucket = S3Bucket(string("s3.image.bucket.name"), imageBucketEndpoint, usesPathStyleURLs = booleanOpt("s3.image.bucket.pathStyleURLs").getOrElse(false), clientFor(imageBucketEndpoint)) + private val thumbBucketEndpoint = string("s3.thumb.bucket.endpoint") + val thumbnailBucket: S3Bucket = S3Bucket(string("s3.thumb.bucket.name"), thumbBucketEndpoint, usesPathStyleURLs = booleanOpt("s3.thumb.bucket.pathStyleURLs").getOrElse(false), clientFor(thumbBucketEndpoint)) /** * Load in a list of domain metadata specifications from configuration. For example: diff --git a/cropper/app/lib/CropperConfig.scala b/cropper/app/lib/CropperConfig.scala index 4a4739d8ce..cbc2b80c95 100644 --- a/cropper/app/lib/CropperConfig.scala +++ b/cropper/app/lib/CropperConfig.scala @@ -11,7 +11,8 @@ class CropperConfig(resources: GridConfigResources) extends CommonConfig(resourc val imgPublishingBucket: S3Bucket = S3Bucket( string("publishing.image.bucket.name"), string("publishing.image.bucket.endpoint"), - boolean("publishing.image.bucket.pathStyleURLs") + boolean("publishing.image.bucket.pathStyleURLs"), + clientFor(string("publishing.image.bucket.endpoint")) ) val canDownloadCrop: Boolean = boolean("canDownloadCrop") diff --git a/cropper/test/lib/CropsTest.scala b/cropper/test/lib/CropsTest.scala index 6a0a64e49c..83fdeb8590 100644 --- a/cropper/test/lib/CropsTest.scala +++ b/cropper/test/lib/CropsTest.scala @@ -1,5 +1,6 @@ package lib +import com.amazonaws.services.s3.AmazonS3 import com.gu.mediaservice.lib.aws.{S3, S3Bucket} import com.gu.mediaservice.lib.imaging.ImageOperations import com.gu.mediaservice.model._ @@ -58,7 +59,8 @@ class CropsTest extends AnyFunSpec with Matchers with MockitoSugar { private val source: SourceImage = SourceImage("test", mock[Asset], valid = true, mock[ImageMetadata], mock[FileMetadata]) private val bounds: Bounds = Bounds(10, 20, 30, 40) private val outputWidth = 1234 - private val imageBucket = S3Bucket("crops-bucket", S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false) + private val mockS3Client = mock[AmazonS3] + private val imageBucket = S3Bucket("crops-bucket", S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false, mockS3Client) private val s3 = new S3(config) it("should should construct a correct address for a master jpg") { diff --git a/image-loader/app/lib/ImageLoaderConfig.scala b/image-loader/app/lib/ImageLoaderConfig.scala index a214d82845..310c006b65 100644 --- a/image-loader/app/lib/ImageLoaderConfig.scala +++ b/image-loader/app/lib/ImageLoaderConfig.scala @@ -15,8 +15,9 @@ class ImageLoaderConfig(resources: GridConfigResources) extends CommonConfig(res val maybeImageReplicaBucket: Option[String] = stringOpt("s3.image.replicaBucket") + private val quarantineBucketEndpoint = S3.AmazonAwsS3Endpoint val quarantineBucket: Option[S3Bucket] = stringOpt("s3.quarantine.bucket").map { bucket => - S3Bucket(bucket, S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false) + S3Bucket(bucket, quarantineBucketEndpoint, usesPathStyleURLs = false, clientFor(quarantineBucketEndpoint)) } val uploadToQuarantineEnabled: Boolean = boolean("upload.quarantine.enabled") diff --git a/image-loader/test/scala/model/ImageUploadTest.scala b/image-loader/test/scala/model/ImageUploadTest.scala index adcb572978..0d894a90fe 100644 --- a/image-loader/test/scala/model/ImageUploadTest.scala +++ b/image-loader/test/scala/model/ImageUploadTest.scala @@ -1,11 +1,8 @@ package model +import com.amazonaws.services.s3.AmazonS3 import com.drew.imaging.ImageProcessingException -import com.gu.mediaservice.lib.aws.{S3Metadata, S3Object, S3ObjectMetadata} -import com.gu.mediaservice.lib.{StorableImage, StorableOptimisedImage, StorableOriginalImage, StorableThumbImage} -import com.gu.mediaservice.lib.aws.{EmbedderMessage, S3Metadata, S3Object, S3ObjectMetadata, S3Ops} -import com.gu.mediaservice.lib.aws.{S3, S3Bucket, S3Metadata, S3Object, S3ObjectMetadata, S3Ops} -import com.gu.mediaservice.lib.aws.{S3Metadata, S3Object, S3ObjectMetadata, S3Ops} +import com.gu.mediaservice.lib.aws.{S3, S3Bucket, S3Object} import com.gu.mediaservice.lib.cleanup.ImageProcessor import com.gu.mediaservice.lib.imaging.ImageOperations import com.gu.mediaservice.lib.logging.LogMarker @@ -21,7 +18,6 @@ import org.scalatestplus.mockito.MockitoSugar import test.lib.ResourceHelpers import java.io.File -import java.net.URI import java.util.UUID import scala.concurrent.{ExecutionContext, Future} import scala.util.{Failure, Success} @@ -33,10 +29,12 @@ class ImageUploadTest extends AsyncFunSuite with Matchers with MockitoSugar { override def markerContents: Map[String, Any] = Map() } + private val mockS3Client = mock[AmazonS3] + private implicit val logMarker: MockLogMarker = new MockLogMarker() // For mime type info, see https://github.com/guardian/grid/pull/2568 val tempDir = new File("/tmp") - val mockConfig: ImageUploadOpsCfg = ImageUploadOpsCfg(tempDir, 256, 85d, List(Tiff), S3Bucket("img-bucket", S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false), S3Bucket("thumb-bucket", S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false)) + val mockConfig: ImageUploadOpsCfg = ImageUploadOpsCfg(tempDir, 256, 85d, List(Tiff), S3Bucket("img-bucket", S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false, mockS3Client), S3Bucket("thumb-bucket", S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false, mockS3Client)) /** * @todo: I flailed about until I found a path that worked, but @@ -52,12 +50,9 @@ class ImageUploadTest extends AsyncFunSuite with Matchers with MockitoSugar { val randomId = UUID.randomUUID().toString + fileName - val mockS3Meta = S3Metadata(Map.empty, S3ObjectMetadata(None, None, None)) - val mockS3Object = S3Object(new URI("innernets.com"), 12345, mockS3Meta) - def mockStore = (a: StorableImage) => Future.successful( - S3Object(S3Bucket("madeupname", S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false), "madeupkey", a.file, Some(a.mimeType), None, a.meta, None) + S3Object(S3Bucket("madeupname", S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false, mockS3Client), "madeupkey", a.file, Some(a.mimeType), None, a.meta, None) ) def storeOrProjectOriginalFile: StorableOriginalImage => Future[S3Object] = mockStore diff --git a/image-loader/test/scala/model/ProjectorTest.scala b/image-loader/test/scala/model/ProjectorTest.scala index 89e77c1b98..e7eceb3fb0 100644 --- a/image-loader/test/scala/model/ProjectorTest.scala +++ b/image-loader/test/scala/model/ProjectorTest.scala @@ -1,8 +1,10 @@ package model +import com.amazonaws.services.s3.AmazonS3 + import java.io.File import java.net.URI -import java.util.{Date, UUID} +import java.util.Date import com.amazonaws.services.s3.model.ObjectMetadata import com.gu.mediaservice.GridClient import com.gu.mediaservice.lib.auth.Authentication @@ -24,7 +26,6 @@ import org.scalatest.time.{Millis, Span} import org.scalatestplus.mockito.MockitoSugar import play.api.libs.json.{JsArray, JsString} import software.amazon.awssdk.services.s3vectors.model.PutVectorsResponse -import play.api.mvc.RequestHeader import test.lib.ResourceHelpers import java.nio.file.Path @@ -42,7 +43,8 @@ class ProjectorTest extends AnyFreeSpec with Matchers with ScalaFutures with Moc private val imageOperations = new ImageOperations(ctxPath) - private val config = ImageUploadOpsCfg(new File("/tmp"), 256, 85d, Nil, S3Bucket("img-bucket", S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false), S3Bucket("thumb-bucket", S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false)) + private val mockS3Client = mock[AmazonS3] + private val config = ImageUploadOpsCfg(new File("/tmp"), 256, 85d, Nil, S3Bucket("img-bucket", S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false, mockS3Client), S3Bucket("thumb-bucket", S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false, mockS3Client)) private val maybeEmbedder = None diff --git a/media-api/app/lib/MediaApiConfig.scala b/media-api/app/lib/MediaApiConfig.scala index 79c312c938..f23f72389f 100644 --- a/media-api/app/lib/MediaApiConfig.scala +++ b/media-api/app/lib/MediaApiConfig.scala @@ -20,17 +20,20 @@ case class StoreConfig( ) class MediaApiConfig(resources: GridConfigResources) extends CommonConfigWithElastic(resources) { - val configBucket: S3Bucket = S3Bucket(string("s3.config.bucket"), S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false) - val usageMailBucket: S3Bucket = S3Bucket(string("s3.usagemail.bucket"), S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false) + private val configBucketEndpoint = S3.AmazonAwsS3Endpoint + val configBucket: S3Bucket = S3Bucket(string("s3.config.bucket"), configBucketEndpoint, usesPathStyleURLs = false, clientFor(configBucketEndpoint)) + private val usageMailBucketEndpoint = S3.AmazonAwsS3Endpoint + val usageMailBucket: S3Bucket = S3Bucket(string("s3.usagemail.bucket"), S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false, clientFor(usageMailBucketEndpoint)) val quotaStoreKey: String = string("quota.store.key") val quotaStoreConfig: StoreConfig = StoreConfig(configBucket, quotaStoreKey) //Lazy allows this to be empty and not break things unless used somewhere - lazy val imgPublishingBucket: S3Bucket = S3Bucket(string("publishing.image.bucket"), S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false) + // TODO this needs to be the same as the crops bucket? Is downloadExports broken? + lazy val imgPublishingBucket: S3Bucket = S3Bucket(string("publishing.image.bucket"), S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false, clientFor(S3.AmazonAwsS3Endpoint)) val cloudFrontDomainThumbBucket: Option[String] = stringOpt("cloudfront.domain.thumbbucket") - val cloudFrontPrivateKeyBucket: Option[S3Bucket] = stringOpt("cloudfront.private-key.bucket").map(S3Bucket(_, S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false)) + val cloudFrontPrivateKeyBucket: Option[S3Bucket] = stringOpt("cloudfront.private-key.bucket").map(S3Bucket(_, S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false, clientFor(S3.AmazonAwsS3Endpoint))) val cloudFrontPrivateKeyBucketKey: Option[String] = stringOpt("cloudfront.private-key.key") val cloudFrontKeyPairId: Option[String] = stringOpt("cloudfront.keypair.id") diff --git a/rest-lib/src/main/scala/com/gu/mediaservice/lib/auth/provider/ApiKeyAuthenticationProvider.scala b/rest-lib/src/main/scala/com/gu/mediaservice/lib/auth/provider/ApiKeyAuthenticationProvider.scala index 87079a4fac..c7b0c0f7b8 100644 --- a/rest-lib/src/main/scala/com/gu/mediaservice/lib/auth/provider/ApiKeyAuthenticationProvider.scala +++ b/rest-lib/src/main/scala/com/gu/mediaservice/lib/auth/provider/ApiKeyAuthenticationProvider.scala @@ -1,10 +1,9 @@ package com.gu.mediaservice.lib.auth.provider import com.gu.mediaservice.lib.auth.Authentication.{MachinePrincipal, Principal} -import com.gu.mediaservice.lib.auth.provider.ApiKeyAuthenticationProvider.{ApiKeyInstance, KindeIdKey} +import com.gu.mediaservice.lib.auth.provider.ApiKeyAuthenticationProvider.ApiKeyInstance import com.gu.mediaservice.lib.auth.{ApiAccessor, KeyStore} -import com.gu.mediaservice.lib.aws.{S3, S3Bucket} +import com.gu.mediaservice.lib.aws.{S3, S3Bucket, S3Ops} import com.gu.mediaservice.lib.config.InstanceForRequest -import com.gu.mediaservice.lib.events.UsageEvents import com.gu.mediaservice.model.Instance import com.typesafe.scalalogging.StrictLogging import play.api.Configuration @@ -26,7 +25,9 @@ class ApiKeyAuthenticationProvider(configuration: Configuration, resources: Auth var keyStorePlaceholder: Option[KeyStore] = _ override def initialise(): Unit = { - val authKeyStoreBucket = S3Bucket(configuration.get[String]("authKeyStoreBucket"), S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false) + val authBucketEndPoint = S3.AmazonAwsS3Endpoint + val authBucketS3Client = S3Ops.buildS3Client(resources.commonConfig) + val authKeyStoreBucket = S3Bucket(configuration.get[String]("authKeyStoreBucket"), authBucketEndPoint, usesPathStyleURLs = false, authBucketS3Client) val store = new KeyStore(authKeyStoreBucket, resources.commonConfig) store.scheduleUpdates(resources.actorSystem.scheduler) keyStorePlaceholder = Some(store) diff --git a/rest-lib/src/test/scala/com/gu/mediaservice/lib/auth/ApiKeyAuthenticationProviderTest.scala b/rest-lib/src/test/scala/com/gu/mediaservice/lib/auth/ApiKeyAuthenticationProviderTest.scala index 76c2bfd1b4..8fcc8ef54c 100644 --- a/rest-lib/src/test/scala/com/gu/mediaservice/lib/auth/ApiKeyAuthenticationProviderTest.scala +++ b/rest-lib/src/test/scala/com/gu/mediaservice/lib/auth/ApiKeyAuthenticationProviderTest.scala @@ -1,5 +1,6 @@ package com.gu.mediaservice.lib.auth +import com.amazonaws.services.s3.AmazonS3 import org.apache.pekko.actor.ActorSystem import com.gu.mediaservice.lib.auth.Authentication.MachinePrincipal import com.gu.mediaservice.lib.auth.provider.{ApiKeyAuthenticationProvider, Authenticated, AuthenticationProviderResources, Invalid, NotAuthenticated, NotAuthorised} @@ -44,7 +45,8 @@ class ApiKeyAuthenticationProviderTest extends AsyncFreeSpec with Matchers with Future.successful(()) } - override def keyStore: KeyStore = new KeyStore(S3Bucket("not-used", S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false), resources.commonConfig) { + val mockS3Client: AmazonS3 = mock[AmazonS3] + override def keyStore: KeyStore = new KeyStore(S3Bucket("not-used", S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false, mockS3Client), resources.commonConfig) { override def lookupIdentity(key: String)(implicit instance: Instance): Option[ApiAccessor] = { key match { case "key-chuckle" => Some(ApiAccessor("brothers", Internal)) diff --git a/thrall/app/lib/ThrallConfig.scala b/thrall/app/lib/ThrallConfig.scala index 48ab0dd971..c6a816136d 100644 --- a/thrall/app/lib/ThrallConfig.scala +++ b/thrall/app/lib/ThrallConfig.scala @@ -56,7 +56,7 @@ object KinesisReceiverConfig { } class ThrallConfig(resources: GridConfigResources) extends CommonConfigWithElastic(resources) { - val maybeReaperBucket: Option[S3Bucket] = stringOpt("s3.reaper.bucket").map(S3Bucket(_, S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false)) + val maybeReaperBucket: Option[S3Bucket] = stringOpt("s3.reaper.bucket").map(S3Bucket(_, S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false, clientFor(S3.AmazonAwsS3Endpoint))) val maybeReaperCountPerRun: Option[Int] = intOpt("reaper.countPerRun") val metadataTopicArn: String = string("indexed.image.sns.topic.arn") From 198f4f4746a9a38105fce0bcd32eceba04a3a113 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sat, 21 Feb 2026 22:20:02 +0000 Subject: [PATCH 36/39] Overly verbose fileMarkers included the all the bucket client's fields. --- common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala index a4266ff887..5bd2b9dacd 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala @@ -157,7 +157,7 @@ class S3(config: CommonConfig) extends GridLogging with ContentDisposition with metadata.setUserMetadata(meta.asJava) val fileMarkers = Map( - "bucket" -> bucket, + "bucket" -> bucket.bucket, "fileName" -> id, "mimeType" -> mimeType.getOrElse("none"), ) From 4a0aa0162f71907535d879b069bb84c9b4a596fa Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Mon, 9 Feb 2026 20:27:07 +0000 Subject: [PATCH 37/39] [gcp-storage-buckets] Crops bucket references from media-api for downloadExport. --- cropper/app/lib/CropperConfig.scala | 1 + media-api/app/lib/MediaApiConfig.scala | 9 +++++++-- media-api/test/lib/elasticsearch/Fixtures.scala | 4 +++- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/cropper/app/lib/CropperConfig.scala b/cropper/app/lib/CropperConfig.scala index cbc2b80c95..5e082585bb 100644 --- a/cropper/app/lib/CropperConfig.scala +++ b/cropper/app/lib/CropperConfig.scala @@ -8,6 +8,7 @@ import java.io.File class CropperConfig(resources: GridConfigResources) extends CommonConfig(resources) { + // TODO this is common with media-api download exports val imgPublishingBucket: S3Bucket = S3Bucket( string("publishing.image.bucket.name"), string("publishing.image.bucket.endpoint"), diff --git a/media-api/app/lib/MediaApiConfig.scala b/media-api/app/lib/MediaApiConfig.scala index f23f72389f..b506508fba 100644 --- a/media-api/app/lib/MediaApiConfig.scala +++ b/media-api/app/lib/MediaApiConfig.scala @@ -28,9 +28,14 @@ class MediaApiConfig(resources: GridConfigResources) extends CommonConfigWithEla val quotaStoreKey: String = string("quota.store.key") val quotaStoreConfig: StoreConfig = StoreConfig(configBucket, quotaStoreKey) - //Lazy allows this to be empty and not break things unless used somewhere // TODO this needs to be the same as the crops bucket? Is downloadExports broken? - lazy val imgPublishingBucket: S3Bucket = S3Bucket(string("publishing.image.bucket"), S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false, clientFor(S3.AmazonAwsS3Endpoint)) + // TODO can be optional at BBC? + val imgPublishingBucket: S3Bucket = S3Bucket( + string("publishing.image.bucket.name"), + string("publishing.image.bucket.endpoint"), + boolean("publishing.image.bucket.pathStyleURLs"), + clientFor(string("publishing.image.bucket.endpoint")) + ) val cloudFrontDomainThumbBucket: Option[String] = stringOpt("cloudfront.domain.thumbbucket") val cloudFrontPrivateKeyBucket: Option[S3Bucket] = stringOpt("cloudfront.private-key.bucket").map(S3Bucket(_, S3.AmazonAwsS3Endpoint, usesPathStyleURLs = false, clientFor(S3.AmazonAwsS3Endpoint))) diff --git a/media-api/test/lib/elasticsearch/Fixtures.scala b/media-api/test/lib/elasticsearch/Fixtures.scala index fed9e13f6c..d6e23a8e0d 100644 --- a/media-api/test/lib/elasticsearch/Fixtures.scala +++ b/media-api/test/lib/elasticsearch/Fixtures.scala @@ -43,7 +43,9 @@ trait Fixtures { "grid.appName", "instance.service.my", "instance.service.instances", - "usageEvents.queue.name" + "usageEvents.queue.name", + "publishing.image.bucket.name", + "publishing.image.bucket.endpoint" ) def deletionData(deletedBy: String): SoftDeletedMetadata = SoftDeletedMetadata( From 209d0ef4912694638282aded2ebfe2ac3b1283e1 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Wed, 25 Feb 2026 12:15:02 +0000 Subject: [PATCH 38/39] [gcp-storage-buckets] uploadToQuarantineEnabled unused --- image-loader/app/lib/ImageLoaderConfig.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/image-loader/app/lib/ImageLoaderConfig.scala b/image-loader/app/lib/ImageLoaderConfig.scala index 310c006b65..6e8bfde3a6 100644 --- a/image-loader/app/lib/ImageLoaderConfig.scala +++ b/image-loader/app/lib/ImageLoaderConfig.scala @@ -19,7 +19,6 @@ class ImageLoaderConfig(resources: GridConfigResources) extends CommonConfig(res val quarantineBucket: Option[S3Bucket] = stringOpt("s3.quarantine.bucket").map { bucket => S3Bucket(bucket, quarantineBucketEndpoint, usesPathStyleURLs = false, clientFor(quarantineBucketEndpoint)) } - val uploadToQuarantineEnabled: Boolean = boolean("upload.quarantine.enabled") val lowerEnvironmentSamplingPercentageAsDecimal = intOpt("s3.sampling.percentage").getOrElse(1) / 100.0 val maybeLowerEnvironmentQueueBucketToSampleInto = stringOpt("s3.sampling.targetBucket") From 8a3d23222c8360b507a9d247338eb8a2a571bb14 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Thu, 7 May 2026 21:59:46 +0100 Subject: [PATCH 39/39] Rebase - revert to origin. --- common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala index 5bd2b9dacd..07a4d77f51 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala @@ -260,11 +260,12 @@ object S3Ops { def buildS3Client(config: CommonConfig, localstackAware: Boolean = true, maybeRegionOverride: Option[String] = None): AmazonS3 = { val builder = config.awsLocalEndpoint match { - case Some(_) if config.isDev => + case Some(_) if config.isDev => { // TODO revise closer to the time of deprecation https://aws.amazon.com/blogs/aws/amazon-s3-path-deprecation-plan-the-rest-of-the-story/ // `withPathStyleAccessEnabled` for localstack // see https://github.com/localstack/localstack/issues/1512 AmazonS3ClientBuilder.standard().withPathStyleAccessEnabled(true) + } case _ => AmazonS3ClientBuilder.standard() }