From 6a8c92cc0c648befae45e18579fff64ab1bbf19b Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sat, 20 Sep 2025 16:57:54 +0100 Subject: [PATCH 01/27] Fixes FileMetadataReaderTest fails locally during British summer by removing non time zoned date formatter which was shadowing the long standing time zoned date formatter. Add a British summer time YYYY-MM-dd example to show that this formatter is locale dependant. DateTimeFormat.forPattern("yyyy-MM-dd") matches the same pattern as ISODateTimeFormat.date.withZoneUTC but removes the withZoneUTC behaviour. I do not know if that was intentional but FileMetadataReaderTest was a long standing test so this could be considered a regression. Additionally, that entire block of date formatters probably have an indeterminate outcome. --- .../gu/mediaservice/lib/metadata/ImageMetadataConverter.scala | 2 +- .../mediaservice/lib/metadata/ImageMetadataConverterTest.scala | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/metadata/ImageMetadataConverter.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/metadata/ImageMetadataConverter.scala index 0e73246752..a8b13ec6b9 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/metadata/ImageMetadataConverter.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/metadata/ImageMetadataConverter.scala @@ -144,13 +144,13 @@ object ImageMetadataConverter extends GridLogging { DateTimeFormat.forPattern("E MMM dd HH:mm:ss.SSS 'BST' yyyy").withZone(DateTimeZone.forOffsetHours(1)), DateTimeFormat.forPattern("E MMM dd HH:mm:ss 'BST' yyyy").withZone(DateTimeZone.forOffsetHours(1)), + // TODO these group of formatters are locale dependent. Is this intentional? DateTimeFormat.forPattern("yyyyMMdd"), DateTimeFormat.forPattern("yyyyMM"), DateTimeFormat.forPattern("yyyyddMM"), DateTimeFormat.forPattern("yyyy"), DateTimeFormat.forPattern("yyyy-MM"), DateTimeFormat.forPattern("yyyy:MM:dd"), - DateTimeFormat.forPattern("yyyy-MM-dd"), // 2014-12-16 - Maybe it's just a date // no timezone provided so force UTC rather than use the machine's timezone diff --git a/common-lib/src/test/scala/com/gu/mediaservice/lib/metadata/ImageMetadataConverterTest.scala b/common-lib/src/test/scala/com/gu/mediaservice/lib/metadata/ImageMetadataConverterTest.scala index 792f38879c..8f0e6d2142 100644 --- a/common-lib/src/test/scala/com/gu/mediaservice/lib/metadata/ImageMetadataConverterTest.scala +++ b/common-lib/src/test/scala/com/gu/mediaservice/lib/metadata/ImageMetadataConverterTest.scala @@ -340,6 +340,7 @@ class ImageMetadataConverterTest extends AnyFunSpec with Matchers { it("should clean up 'just date' dates into iso format") { ImageMetadataConverter.cleanDate("2014-12-16") shouldBe "2014-12-16T00:00:00.000Z" + ImageMetadataConverter.cleanDate("2014-08-20") shouldBe "2014-08-20T00:00:00.000Z" } it("should clean up iso dates with seconds into iso format") { From 2ac26b6c8b5206e871867be2345841a3d1dc12d4 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Fri, 22 Nov 2024 22:39:44 +0000 Subject: [PATCH 02/27] CropController uses GridClient for it's get source image call to Media API. This call was probably the only service to service call circa 2021. GridClient appears to be how more recent service to service calls are done. Moving this call to GridClient helps to enclosure all the service to service url concerns in one place. Get SourceImage requests additional fields via media-api query parameters. Extract the media api uri to image id extraction to a function for testing. --- .../com/gu/mediaservice/GridClient.scala | 19 ++++++-- cropper/app/CropperComponents.scala | 5 ++- .../app/controllers/CropperController.scala | 44 +++---------------- cropper/app/controllers/MediaApiUrls.scala | 16 +++++++ .../test/controllers/MediaApiUrlsTest.scala | 22 ++++++++++ 5 files changed, 64 insertions(+), 42 deletions(-) create mode 100644 cropper/app/controllers/MediaApiUrls.scala create mode 100644 cropper/test/controllers/MediaApiUrlsTest.scala diff --git a/common-lib/src/main/scala/com/gu/mediaservice/GridClient.scala b/common-lib/src/main/scala/com/gu/mediaservice/GridClient.scala index 58373d2333..5a991b3541 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/GridClient.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/GridClient.scala @@ -3,7 +3,7 @@ package com.gu.mediaservice import java.net.URL import com.gu.mediaservice.GridClient.{Error, Found, NotFound, Response} import com.gu.mediaservice.lib.config.Services -import com.gu.mediaservice.model.{Collection, Crop, Edits, Image, ImageMetadata, ImageStatusRecord, SyndicationRights} +import com.gu.mediaservice.model.{Collection, Crop, Edits, Image, ImageMetadata, ImageStatusRecord, SourceImage, SyndicationRights} import com.gu.mediaservice.model.leases.LeasesByMedia import com.gu.mediaservice.model.usage.Usage import com.typesafe.scalalogging.LazyLogging @@ -104,12 +104,13 @@ class GridClient(services: Services)(implicit wsClient: WSClient) extends LazyLo * process before returning data. * See also https://www.playframework.com/documentation/2.6.x/ScalaWS#Configuring-Timeouts */ - def makeGetRequestAsync(url: URL, authFn: WSRequest => WSRequest, requestTimeout: Option[Duration] = None) + def makeGetRequestAsync(url: URL, authFn: WSRequest => WSRequest, requestTimeout: Option[Duration] = None, + queryStringParameters: Option[Seq[(String, String)]] = None) (implicit ec: ExecutionContext): Future[Response] = { - val request: WSRequest = wsClient.url(url.toString) + val request: WSRequest = wsClient.url(url.toString).withQueryStringParameters(queryStringParameters.getOrElse(Seq.empty): _*) val requestWithTimeout = requestTimeout.fold(request)(request.withRequestTimeout) val authorisedRequest = authFn(requestWithTimeout) - authorisedRequest.get().map { response => validateResponse(response, url)} + authorisedRequest.get().map { response => validateResponse(response, url) } } private def validateResponse( @@ -234,6 +235,16 @@ class GridClient(services: Services)(implicit wsClient: WSClient) extends LazyLo } } + def getSourceImage(mediaId: String, authFn: WSRequest => WSRequest)(implicit ec: ExecutionContext): Future[SourceImage] = { + logger.info("attempt to get image") + val url = new URL(s"${services.apiBaseUri}/images/$mediaId") + makeGetRequestAsync(url, authFn, queryStringParameters = Some(Seq("include" -> "fileMetadata"))) map { + case Found(json, _) => json.as[SourceImage] + case nf@NotFound(_, _) => Error(nf.status, url, nf.underlying).logErrorAndThrowException() + case e@Error(_, _, _) => e.logErrorAndThrowException() + } + } + def getMetadata(mediaId: String, authFn: WSRequest => WSRequest)(implicit ec: ExecutionContext): Future[ImageMetadata] = { logger.info("attempt to get metadata") val url = new URL(s"${services.apiBaseUri}/images/$mediaId") diff --git a/cropper/app/CropperComponents.scala b/cropper/app/CropperComponents.scala index c51f554d36..3770542919 100644 --- a/cropper/app/CropperComponents.scala +++ b/cropper/app/CropperComponents.scala @@ -1,3 +1,4 @@ +import com.gu.mediaservice.GridClient import com.gu.mediaservice.lib.imaging.ImageOperations import com.gu.mediaservice.lib.management.{InnerServiceStatusCheckController, Management} import com.gu.mediaservice.lib.play.GridComponents @@ -15,7 +16,9 @@ class CropperComponents(context: Context) extends GridComponents(context, new Cr val crops = new Crops(config, store, imageOperations) val notifications = new Notifications(config) - val controller = new CropperController(auth, crops, store, notifications, config, controllerComponents, wsClient, authorisation) + private val gridClient = GridClient(config.services)(wsClient) + + val controller = new CropperController(auth, crops, store, notifications, config, controllerComponents, authorisation, gridClient) val permissionsAwareManagement = new Management(controllerComponents, buildInfo) val InnerServiceStatusCheckController = new InnerServiceStatusCheckController(auth, controllerComponents, config.services, wsClient) diff --git a/cropper/app/controllers/CropperController.scala b/cropper/app/controllers/CropperController.scala index dc2f05aff7..0d6a1211df 100644 --- a/cropper/app/controllers/CropperController.scala +++ b/cropper/app/controllers/CropperController.scala @@ -2,6 +2,7 @@ package controllers import _root_.play.api.libs.json._ import _root_.play.api.mvc.{BaseController, ControllerComponents} +import com.gu.mediaservice.GridClient import com.gu.mediaservice.lib.argo.ArgoHelpers import com.gu.mediaservice.lib.argo.model.Link import com.gu.mediaservice.lib.auth.Authentication.Principal @@ -16,7 +17,6 @@ import com.gu.mediaservice.syntax.MessageSubjects import lib._ import model._ import org.joda.time.DateTime -import play.api.libs.ws.WSClient import java.net.URI import scala.concurrent.{ExecutionContext, Future} @@ -30,8 +30,9 @@ case object ApiRequestFailed extends Exception("Failed to fetch the source") class CropperController(auth: Authentication, crops: Crops, store: CropStore, notifications: Notifications, config: CropperConfig, override val controllerComponents: ControllerComponents, - ws: WSClient, authorisation: Authorisation)(implicit val ec: ExecutionContext) - extends BaseController with MessageSubjects with ArgoHelpers { + authorisation: Authorisation, + gridClient: GridClient)(implicit val ec: ExecutionContext) + extends BaseController with MessageSubjects with ArgoHelpers with MediaApiUrls { // Stupid name clash between Argo and Play import com.gu.mediaservice.lib.argo.model.{Action => ArgoAction} @@ -163,7 +164,7 @@ class CropperController(auth: Authentication, crops: Crops, store: CropStore, no )(implicit logMarker: LogMarker): Future[(String, Crop)] = { for { - _ <- verify(isMediaApiUri(exportRequest.uri), InvalidSource) + _ <- verify(isMediaApiImageUri(exportRequest.uri, config.apiUri), InvalidSource) apiImage <- fetchSourceFromApi(exportRequest.uri, onBehalfOfPrincipal) _ <- verify(apiImage.valid, InvalidImage) // Image should always have dimensions, but we want to safely extract the Option @@ -183,39 +184,8 @@ class CropperController(auth: Authentication, crops: Crops, store: CropStore, no } yield (id, finalCrop) } - // TODO: lame, parse into URI object and compare host instead - def isMediaApiUri(uri: String): Boolean = uri.startsWith(config.apiUri) - - def fetchSourceFromApi(uri: String, onBehalfOfPrincipal: Authentication.OnBehalfOfPrincipal): Future[SourceImage] = { - - case class HttpClientResponse(status: Int, statusText: String, json: JsValue) - - val baseRequest = ws.url(uri) - .withQueryStringParameters("include" -> "fileMetadata") - - val request = onBehalfOfPrincipal(baseRequest) - - val responseFuture = request.get().map { r => - HttpClientResponse(r.status, r.statusText, Json.parse(r.body)) - } - - responseFuture recoverWith { - case NonFatal(e) => - logger.warn(s"HTTP request to fetch source failed: $e") - Future.failed(ApiRequestFailed) - } - - for (resp <- responseFuture) - yield { - if (resp.status == 404) { - throw ImageNotFound - } else if (resp.status != 200) { - logger.warn(s"HTTP status ${resp.status} ${resp.statusText} from $uri") - throw ApiRequestFailed - } else { - resp.json.as[SourceImage] - } - } + private def fetchSourceFromApi(uri: String, onBehalfOfPrincipal: Authentication.OnBehalfOfPrincipal): Future[SourceImage] = { + gridClient.getSourceImage(imageIdFrom(uri), onBehalfOfPrincipal) } def verify(cond: => Boolean, error: Throwable): Future[Unit] = diff --git a/cropper/app/controllers/MediaApiUrls.scala b/cropper/app/controllers/MediaApiUrls.scala new file mode 100644 index 0000000000..5ccb8f4161 --- /dev/null +++ b/cropper/app/controllers/MediaApiUrls.scala @@ -0,0 +1,16 @@ +package controllers + +trait MediaApiUrls { + + def isMediaApiImageUri(uri: String, apiUri: String): Boolean = { + val hasMediaApiPrefix = uri.startsWith(apiUri) + val suffix = uri.drop(apiUri.length) + val suffixComponents = suffix.split("/") + val hasImageSuffix = suffixComponents.length == 3 && suffixComponents(1) == "images" + hasMediaApiPrefix && hasImageSuffix + } + + def imageIdFrom(uri: String): String = { + uri.split("/").last + } +} diff --git a/cropper/test/controllers/MediaApiUrlsTest.scala b/cropper/test/controllers/MediaApiUrlsTest.scala new file mode 100644 index 0000000000..6033c6b8ab --- /dev/null +++ b/cropper/test/controllers/MediaApiUrlsTest.scala @@ -0,0 +1,22 @@ +package controllers + +import com.gu.mediaservice.lib.imaging.ImageOperations +import com.gu.mediaservice.model._ +import org.scalatest.funspec.AnyFunSpec +import org.scalatest.matchers.should.Matchers +import org.scalatestplus.mockito.MockitoSugar +import org.scalatest._ +import flatspec._ +import matchers._ + +class MediaApiUrlsTest extends AnyFlatSpec with Matchers with MediaApiUrls { + + "media api urls" should + "identify media API image urls" in { + isMediaApiImageUri("https://media.api.test.com/images/cb5b8c05b690db2d034457b5461ef32abf29eff8", "https://media.api.test.com") shouldBe true + isMediaApiImageUri("https://media.api.test.com/images", "https://media.api.test.com") shouldBe false + isMediaApiImageUri("images/cb5b8c05b690db2d034457b5461ef32abf29eff8", "https://media.api.test.com") shouldBe false + isMediaApiImageUri("cb5b8c05b690db2d034457b5461ef32abf29eff8", "https://media.api.test.com") shouldBe false + } + +} From d9d7f1c81fe67e52eeee320e9d6d33d1d11cf78e Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 5 May 2024 15:45:28 +0100 Subject: [PATCH 03/27] Neuter CloudWatchMetrics; TODO push to config. --- .../com/gu/mediaservice/lib/metrics/CloudWatchMetrics.scala | 2 ++ 1 file changed, 2 insertions(+) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/metrics/CloudWatchMetrics.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/metrics/CloudWatchMetrics.scala index 572ae8ea49..d1acae2bab 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/metrics/CloudWatchMetrics.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/metrics/CloudWatchMetrics.scala @@ -104,9 +104,11 @@ private class MetricsActor(namespace: String, client: AmazonCloudWatch) extends .toSeq aggregatedMetrics.grouped(maxGroupSize).foreach(chunkedMetrics => { //can only send max 20 metrics to CW at a time + /* Yeah nah client.putMetricData(new PutMetricDataRequest() .withNamespace(namespace) .withMetricData(chunkedMetrics.asJava)) + */ }) logger.info(s"Put ${data.size} metric data points (aggregated to ${aggregatedMetrics.size} points) to namespace $namespace") From b764c69d76bf375eb97e0fd7818d4e73264aae0c Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 5 May 2024 15:59:23 +0100 Subject: [PATCH 04/27] Want to disable Kinises client's noisey CloudWatch emissions. --- thrall/app/lib/ThrallConfig.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/thrall/app/lib/ThrallConfig.scala b/thrall/app/lib/ThrallConfig.scala index a5eeab49d8..4385661961 100644 --- a/thrall/app/lib/ThrallConfig.scala +++ b/thrall/app/lib/ThrallConfig.scala @@ -27,7 +27,7 @@ case class KinesisReceiverConfig( override val isDev: Boolean, streamName: String, rewindFrom: Option[DateTime], - metricsLevel: MetricsLevel = MetricsLevel.DETAILED + metricsLevel: MetricsLevel = MetricsLevel.NONE ) extends AwsClientV2BuilderUtils { lazy val kinesisClient: KinesisAsyncClient = { val clientBuilder = withAWSCredentialsV2(KinesisAsyncClient.builder()) From c5326adb68174079f548ebd18f957116200ea79c Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 5 May 2024 14:06:52 +0100 Subject: [PATCH 05/27] Switch to EnvironmentVariableCredentialsProvider to defer having to work out AWS roles in containers for now. Use only ENV AWS creds; should stop AWS auth endpoint timeouts on first hit. --- .../gu/mediaservice/lib/aws/AwsClientV1BuilderUtils.scala | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/AwsClientV1BuilderUtils.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/AwsClientV1BuilderUtils.scala index cdd4ec04d9..02b3e93d9e 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/AwsClientV1BuilderUtils.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/AwsClientV1BuilderUtils.scala @@ -1,7 +1,7 @@ package com.gu.mediaservice.lib.aws import com.amazonaws.auth.profile.ProfileCredentialsProvider -import com.amazonaws.auth.{AWSCredentialsProvider, AWSCredentialsProviderChain, InstanceProfileCredentialsProvider} +import com.amazonaws.auth.{AWSCredentialsProvider, AWSCredentialsProviderChain, EnvironmentVariableCredentialsProvider, InstanceProfileCredentialsProvider} import com.amazonaws.client.builder.AwsClientBuilder import com.amazonaws.client.builder.AwsClientBuilder.EndpointConfiguration import com.gu.mediaservice.lib.logging.GridLogging @@ -13,8 +13,7 @@ trait AwsClientV1BuilderUtils extends GridLogging { def awsRegion: String = "eu-west-1" def awsCredentials: AWSCredentialsProvider = new AWSCredentialsProviderChain( - new ProfileCredentialsProvider("media-service"), - InstanceProfileCredentialsProvider.getInstance() + new EnvironmentVariableCredentialsProvider(), ) final def awsEndpointConfiguration: Option[EndpointConfiguration] = awsLocalEndpoint match { From c75bd5cba21e23b41a1bcd0eddc85712f8184fe1 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Wed, 29 May 2024 12:27:59 +0100 Subject: [PATCH 06/27] Play secret from ENV; need to explicitly resolve placeholders. com.typesafe.config.ConfigException$NotResolved: need to Config#resolve() each config before using it, see the API docs for Config#resolve() --- .../com/gu/mediaservice/lib/config/GridConfigLoader.scala | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/GridConfigLoader.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/GridConfigLoader.scala index 7484ccf810..1e027209b9 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/GridConfigLoader.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/GridConfigLoader.scala @@ -57,7 +57,10 @@ object GridConfigLoader extends StrictLogging { if (file.getPath.endsWith(".properties")) { logger.warn(s"Configuring the Grid with Java properties files is deprecated as of #3011, please switch to .conf files. See #3037 for a conversion utility.") } - Configuration(ConfigFactory.parseFile(file)) + val parsed = ConfigFactory.parseFile(file) + logger.info(s"Resolving config parsed from file: $file") + val resolved = parsed.resolve() + Configuration(resolved) } else { logger.info(s"Skipping config file $file as it doesn't exist") Configuration.empty From 86aa4a38abec0c2407a4aac7345f16f3bfa767cc Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sat, 4 May 2024 18:49:21 +0100 Subject: [PATCH 07/27] Initial docker image builds sbt universal, then straight to sbt docker may be the correct path now. Play framework assembly docs do not help with Caused by: java.lang.ClassNotFoundException: play.core.server.ProdServerStart Attempt to build thrall as a fat jar using assembly. Reminder of what Thrall does. --- build.sbt | 35 ++++++++++++++++++++++------------- project/plugins.sbt | 2 -- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/build.sbt b/build.sbt index 68f81a84c4..c2a36ea9ae 100644 --- a/build.sbt +++ b/build.sbt @@ -4,8 +4,7 @@ import sbt.Package.FixedTimestamp import scala.sys.process._ import scala.util.control.NonFatal import scala.collection.JavaConverters._ - -import com.typesafe.sbt.packager.debian.JDebPackaging +import com.typesafe.sbt.packager.docker._ // We need to keep the timestamps to allow caching headers to work as expected on assets. // The below should work, but some problem in one of the plugins (possible the play plugin? or sbt-web?) causes @@ -233,10 +232,21 @@ val buildInfo = Seq( ) def playProject(projectName: String, port: Int, path: Option[String] = None): Project = { - val commonProject = project(projectName, path) - .enablePlugins(PlayScala, JDebPackaging, SystemdPlugin, BuildInfoPlugin) + project(projectName, path) + .enablePlugins(PlayScala, BuildInfoPlugin, DockerPlugin) .dependsOn(restLib) .settings(commonSettings ++ buildInfo ++ Seq( + dockerBaseImage := "openjdk:11-jre", + dockerExposedPorts in Docker := Seq(port), + // TODO image-loader specific + dockerCommands ++= Seq( + Cmd("USER", "root"), Cmd("RUN", "apt-get", "update"), + Cmd("RUN", "apt-get", "install", "-y", "apt-utils"), + Cmd("RUN", "apt-get", "install", "-y", "graphicsmagick"), + Cmd("RUN", "apt-get", "install", "-y", "graphicsmagick-imagemagick-compat"), + Cmd("RUN", "apt-get", "install", "-y", "pngquant"), + Cmd("RUN", "apt-get", "install", "-y", "libimage-exiftool-perl") + ), playDefaultPort := port, debianPackageDependencies := Seq("java11-runtime-headless"), Linux / maintainer := "Guardian Developers ", @@ -256,16 +266,15 @@ def playProject(projectName: String, port: Int, path: Option[String] = None): Pr }, Universal / mappings ++= Seq( file("common-lib/src/main/resources/application.conf") -> "conf/application.conf", - file("common-lib/src/main/resources/logback.xml") -> "conf/logback.xml" + file("common-lib/src/main/resources/logback.xml") -> "conf/logback.xml", + // TODO image-loader specific + file("image-loader/facebook-TINYsRGB_c2.icc") -> "facebook-TINYsRGB_c2.icc", + file("image-loader/grayscale.icc") -> "grayscale.icc", + file("image-loader/srgb.icc") -> "srgb.icc" ), Universal / javaOptions ++= Seq( "-Dpidfile.path=/dev/null", - s"-Dconfig.file=/usr/share/$projectName/conf/application.conf", - s"-Dlogger.file=/usr/share/$projectName/conf/logback.xml", - "-J-Xlog:gc*", - s"-J-Xlog:gc:/var/log/$projectName/gc.log" - ) - )) - //Add the BBC library dependency if defined - maybeBBCLib.fold(commonProject){commonProject.dependsOn(_)} + s"-Dconfig.file=/opt/docker/conf/application.conf", + s"-Dlogger.file=/opt/docker/conf/logback.xml" + ))) } diff --git a/project/plugins.sbt b/project/plugins.sbt index 3f3177253f..8ce651e69d 100644 --- a/project/plugins.sbt +++ b/project/plugins.sbt @@ -1,5 +1,3 @@ -libraryDependencies += "org.vafer" % "jdeb" % "1.3" artifacts (Artifact("jdeb", "jar", "jar")) - addSbtPlugin("org.playframework" % "sbt-plugin" % "3.0.5") addSbtPlugin("com.eed3si9n" % "sbt-buildinfo" % "0.9.0") From 7e4c91d08411881aa7446f4b8ef608fe63c3ee3d Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Tue, 7 May 2024 14:55:02 +0100 Subject: [PATCH 08/27] Fork image loader specific play project. cmyk.icc is required for ingesting CMKY colour space JPEGs. --- build.sbt | 45 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 39 insertions(+), 6 deletions(-) diff --git a/build.sbt b/build.sbt index c2a36ea9ae..500d940cd6 100644 --- a/build.sbt +++ b/build.sbt @@ -126,7 +126,7 @@ lazy val collections = playProject("collections", 9010) lazy val cropper = playProject("cropper", 9006) -lazy val imageLoader = playProject("image-loader", 9003).settings { +lazy val imageLoader = playImageLoaderProject("image-loader", 9003).settings { libraryDependencies ++= Seq( "org.apache.tika" % "tika-core" % "1.28.5", "com.drewnoakes" % "metadata-extractor" % "2.19.0" @@ -238,7 +238,41 @@ def playProject(projectName: String, port: Int, path: Option[String] = None): Pr .settings(commonSettings ++ buildInfo ++ Seq( dockerBaseImage := "openjdk:11-jre", dockerExposedPorts in Docker := Seq(port), - // TODO image-loader specific + playDefaultPort := port, + debianPackageDependencies := Seq("java11-runtime-headless"), + Linux / maintainer := "Guardian Developers ", + Linux / packageSummary := description.value, + packageDescription := description.value, + + bashScriptEnvConfigLocation := Some("/etc/environment"), + Debian / makeEtcDefault := None, + Debian / packageBin := { + val originalFileName = (Debian / packageBin).value + val (base, ext) = originalFileName.baseAndExt + val newBase = base.replace(s"_${version.value}_all","") + val newFileName = file(originalFileName.getParent) / s"$newBase.$ext" + IO.move(originalFileName, newFileName) + println(s"Renamed $originalFileName to $newFileName") + newFileName + }, + Universal / mappings ++= Seq( + file("common-lib/src/main/resources/application.conf") -> "conf/application.conf", + file("common-lib/src/main/resources/logback.xml") -> "conf/logback.xml" + ), + Universal / javaOptions ++= Seq( + "-Dpidfile.path=/dev/null", + s"-Dconfig.file=/opt/docker/conf/application.conf", + s"-Dlogger.file=/opt/docker/conf/logback.xml" + ))) +} + +def playImageLoaderProject(projectName: String, port: Int, path: Option[String] = None): Project = { + project(projectName, path) + .enablePlugins(PlayScala, BuildInfoPlugin, DockerPlugin) + .dependsOn(restLib) + .settings(commonSettings ++ buildInfo ++ Seq( + dockerBaseImage := "openjdk:11-jre", + dockerExposedPorts in Docker := Seq(port), dockerCommands ++= Seq( Cmd("USER", "root"), Cmd("RUN", "apt-get", "update"), Cmd("RUN", "apt-get", "install", "-y", "apt-utils"), @@ -248,17 +282,16 @@ def playProject(projectName: String, port: Int, path: Option[String] = None): Pr Cmd("RUN", "apt-get", "install", "-y", "libimage-exiftool-perl") ), playDefaultPort := port, - debianPackageDependencies := Seq("java11-runtime-headless"), + debianPackageDependencies := Seq("openjdk-8-jre-headless"), Linux / maintainer := "Guardian Developers ", Linux / packageSummary := description.value, packageDescription := description.value, - bashScriptEnvConfigLocation := Some("/etc/environment"), Debian / makeEtcDefault := None, Debian / packageBin := { val originalFileName = (Debian / packageBin).value val (base, ext) = originalFileName.baseAndExt - val newBase = base.replace(s"_${version.value}_all","") + val newBase = base.replace(s"_${version.value}_all", "") val newFileName = file(originalFileName.getParent) / s"$newBase.$ext" IO.move(originalFileName, newFileName) println(s"Renamed $originalFileName to $newFileName") @@ -267,7 +300,7 @@ def playProject(projectName: String, port: Int, path: Option[String] = None): Pr Universal / mappings ++= Seq( file("common-lib/src/main/resources/application.conf") -> "conf/application.conf", file("common-lib/src/main/resources/logback.xml") -> "conf/logback.xml", - // TODO image-loader specific + file("image-loader/cmyk.icc") -> "cmyk.icc", file("image-loader/facebook-TINYsRGB_c2.icc") -> "facebook-TINYsRGB_c2.icc", file("image-loader/grayscale.icc") -> "grayscale.icc", file("image-loader/srgb.icc") -> "srgb.icc" From 05a8e162afd7362a7e72c9cd746f48c7179989ca Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Thu, 9 May 2024 15:25:13 +0100 Subject: [PATCH 09/27] Cropper asks for 'gm' so give it the same Debian packages as image-uploader. --- build.sbt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.sbt b/build.sbt index 500d940cd6..811f8d3976 100644 --- a/build.sbt +++ b/build.sbt @@ -124,7 +124,7 @@ lazy val auth = playProject("auth", 9011) lazy val collections = playProject("collections", 9010) -lazy val cropper = playProject("cropper", 9006) +lazy val cropper = playImageLoaderProject("cropper", 9006) lazy val imageLoader = playImageLoaderProject("image-loader", 9003).settings { libraryDependencies ++= Seq( From 572831c79f774b993ef017c4397451bd5d49e313 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sat, 3 May 2025 10:27:53 +0100 Subject: [PATCH 10/27] build.sbt drop Debian package related config which is not needed for image builds. Remove redundant debianPackageDependencies options. --- build.sbt | 33 +++------------------------------ 1 file changed, 3 insertions(+), 30 deletions(-) diff --git a/build.sbt b/build.sbt index 811f8d3976..71daba3055 100644 --- a/build.sbt +++ b/build.sbt @@ -236,25 +236,11 @@ def playProject(projectName: String, port: Int, path: Option[String] = None): Pr .enablePlugins(PlayScala, BuildInfoPlugin, DockerPlugin) .dependsOn(restLib) .settings(commonSettings ++ buildInfo ++ Seq( - dockerBaseImage := "openjdk:11-jre", + dockerBaseImage := "eclipse-temurin:11", dockerExposedPorts in Docker := Seq(port), playDefaultPort := port, - debianPackageDependencies := Seq("java11-runtime-headless"), - Linux / maintainer := "Guardian Developers ", - Linux / packageSummary := description.value, - packageDescription := description.value, bashScriptEnvConfigLocation := Some("/etc/environment"), - Debian / makeEtcDefault := None, - Debian / packageBin := { - val originalFileName = (Debian / packageBin).value - val (base, ext) = originalFileName.baseAndExt - val newBase = base.replace(s"_${version.value}_all","") - val newFileName = file(originalFileName.getParent) / s"$newBase.$ext" - IO.move(originalFileName, newFileName) - println(s"Renamed $originalFileName to $newFileName") - newFileName - }, Universal / mappings ++= Seq( file("common-lib/src/main/resources/application.conf") -> "conf/application.conf", file("common-lib/src/main/resources/logback.xml") -> "conf/logback.xml" @@ -271,7 +257,7 @@ def playImageLoaderProject(projectName: String, port: Int, path: Option[String] .enablePlugins(PlayScala, BuildInfoPlugin, DockerPlugin) .dependsOn(restLib) .settings(commonSettings ++ buildInfo ++ Seq( - dockerBaseImage := "openjdk:11-jre", + dockerBaseImage := "eclipse-temurin:11", dockerExposedPorts in Docker := Seq(port), dockerCommands ++= Seq( Cmd("USER", "root"), Cmd("RUN", "apt-get", "update"), @@ -282,21 +268,8 @@ def playImageLoaderProject(projectName: String, port: Int, path: Option[String] Cmd("RUN", "apt-get", "install", "-y", "libimage-exiftool-perl") ), playDefaultPort := port, - debianPackageDependencies := Seq("openjdk-8-jre-headless"), - Linux / maintainer := "Guardian Developers ", - Linux / packageSummary := description.value, - packageDescription := description.value, + bashScriptEnvConfigLocation := Some("/etc/environment"), - Debian / makeEtcDefault := None, - Debian / packageBin := { - val originalFileName = (Debian / packageBin).value - val (base, ext) = originalFileName.baseAndExt - val newBase = base.replace(s"_${version.value}_all", "") - val newFileName = file(originalFileName.getParent) / s"$newBase.$ext" - IO.move(originalFileName, newFileName) - println(s"Renamed $originalFileName to $newFileName") - newFileName - }, Universal / mappings ++= Seq( file("common-lib/src/main/resources/application.conf") -> "conf/application.conf", file("common-lib/src/main/resources/logback.xml") -> "conf/logback.xml", From 1aa711806e78ff607202161317cbeff8619b15a6 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 5 May 2024 11:48:38 +0100 Subject: [PATCH 11/27] Only log to stdout in the containerised world. --- common-lib/src/main/resources/logback.xml | 31 +---------------------- 1 file changed, 1 insertion(+), 30 deletions(-) diff --git a/common-lib/src/main/resources/logback.xml b/common-lib/src/main/resources/logback.xml index 733a850c11..47e666377f 100644 --- a/common-lib/src/main/resources/logback.xml +++ b/common-lib/src/main/resources/logback.xml @@ -2,30 +2,6 @@ - - - - - - - - - - - ${LOGS_LOCATION}/application.log - - - ${LOGS_LOCATION}/application.log.%d{yyyy-MM-dd}.%i.gz - 100MB - 7 - 500MB - - - - %date - [%level] - from %logger in %thread markers=%marker %n%message%n%xException%n - - - @@ -39,12 +15,7 @@ - - - - - - + From 3b8455a704205d4876e5da81f52402ed6124a9ab Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Wed, 8 May 2024 11:06:11 +0100 Subject: [PATCH 12/27] Alter Syndication access check to use URIs not raw host name; allows all api host name to be internalised. Interface only talks about base URIs. --- .../mediaservice/lib/auth/ApiAccessor.scala | 5 +++- .../gu/mediaservice/lib/config/Services.scala | 25 +++++++++---------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/auth/ApiAccessor.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/auth/ApiAccessor.scala index 3e1924451d..14bd411fa7 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/auth/ApiAccessor.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/auth/ApiAccessor.scala @@ -29,6 +29,9 @@ object ApiAccessor extends ArgoHelpers { def hasAccess(apiKey: ApiAccessor, request: RequestHeader, services: Services): Boolean = apiKey.tier match { case Internal => true case ReadOnly => request.method == "GET" - case Syndication => request.method == "GET" && request.host == services.apiHost && request.path.startsWith("/images") + case Syndication => { + val isMediaApiRequest = request.uri.startsWith(services.apiBaseUri) // TODO check this! + request.method == "GET" && isMediaApiRequest && request.path.startsWith("/images") + } } } diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala index 896acf8549..bf8df4cc66 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala @@ -39,19 +39,18 @@ object ServiceHosts { } class Services(val domainRoot: String, hosts: ServiceHosts, corsAllowedOrigins: Set[String], domainRootOverride: Option[String] = None) { - val kahunaHost: String = s"${hosts.kahunaPrefix}$domainRoot" - val apiHost: String = s"${hosts.apiPrefix}$domainRoot" - val loaderHost: String = s"${hosts.loaderPrefix}${domainRootOverride.getOrElse(domainRoot)}" - val cropperHost: String = s"${hosts.cropperPrefix}${domainRootOverride.getOrElse(domainRoot)}" - val metadataHost: String = s"${hosts.metadataPrefix}${domainRootOverride.getOrElse(domainRoot)}" - val imgopsHost: String = s"${hosts.imgopsPrefix}${domainRootOverride.getOrElse(domainRoot)}" - val usageHost: String = s"${hosts.usagePrefix}${domainRootOverride.getOrElse(domainRoot)}" - val collectionsHost: String = s"${hosts.collectionsPrefix}${domainRootOverride.getOrElse(domainRoot)}" - val leasesHost: String = s"${hosts.leasesPrefix}${domainRootOverride.getOrElse(domainRoot)}" - val authHost: String = s"${hosts.authPrefix}$domainRoot" - val projectionHost: String = s"${hosts.projectionPrefix}${domainRootOverride.getOrElse(domainRoot)}" - val thrallHost: String = s"${hosts.thrallPrefix}${domainRootOverride.getOrElse(domainRoot)}" - + private val kahunaHost: String = s"${hosts.kahunaPrefix}$domainRoot" + private val apiHost: String = s"${hosts.apiPrefix}$domainRoot" + private val loaderHost: String = s"${hosts.loaderPrefix}${domainRootOverride.getOrElse(domainRoot)}" + private val cropperHost: String = s"${hosts.cropperPrefix}${domainRootOverride.getOrElse(domainRoot)}" + private val metadataHost: String = s"${hosts.metadataPrefix}${domainRootOverride.getOrElse(domainRoot)}" + private val imgopsHost: String = s"${hosts.imgopsPrefix}${domainRootOverride.getOrElse(domainRoot)}" + private val usageHost: String = s"${hosts.usagePrefix}${domainRootOverride.getOrElse(domainRoot)}" + private val collectionsHost: String = s"${hosts.collectionsPrefix}${domainRootOverride.getOrElse(domainRoot)}" + private val leasesHost: String = s"${hosts.leasesPrefix}${domainRootOverride.getOrElse(domainRoot)}" + private val authHost: String = s"${hosts.authPrefix}$domainRoot" + private val projectionHost: String = s"${hosts.projectionPrefix}${domainRootOverride.getOrElse(domainRoot)}" + private val thrallHost: String = s"${hosts.thrallPrefix}${domainRootOverride.getOrElse(domainRoot)}" val kahunaBaseUri = baseUri(kahunaHost) val apiBaseUri = baseUri(apiHost) From b2b90ee07c2c27d0426a8ae47efc6ecec39fba8e Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Wed, 8 May 2024 11:20:02 +0100 Subject: [PATCH 13/27] Introduce an interface to document the exposed service uris. Split url Services into a trait and a Guaridan specific implementation; exposes a few 4th wall breaking direct init's in services. --- .../lib/config/CommonConfig.scala | 2 +- .../gu/mediaservice/lib/config/Services.scala | 114 ++++++++++++------ image-loader/app/ImageLoaderComponents.scala | 4 +- media-api/app/controllers/MediaApi.scala | 4 +- .../app/controllers/EditsController.scala | 5 +- thrall/app/ThrallComponents.scala | 4 +- 6 files changed, 85 insertions(+), 48 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 ce25eec81e..621fc7016a 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 @@ -77,7 +77,7 @@ abstract class CommonConfig(resources: GridConfigResources) extends AwsClientV1B val corsAllowedOrigins: Set[String] = getStringSet("security.cors.allowedOrigins") - val services = new Services(domainRoot, serviceHosts, corsAllowedOrigins, domainRootOverride) + val services = new GuardianUrlSchemeServices(domainRoot, serviceHosts, corsAllowedOrigins, domainRootOverride) /** * Load in a list of domain metadata specifications from configuration. For example: diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala index bf8df4cc66..3c7d203167 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala @@ -1,19 +1,57 @@ package com.gu.mediaservice.lib.config +trait Services { + + def kahunaBaseUri: String + + def apiBaseUri: String + + def loaderBaseUri: String + + def projectionBaseUri: String + + def cropperBaseUri: String + + def metadataBaseUri: String + + def imgopsBaseUri: String + + def usageBaseUri: String + + def collectionsBaseUri: String + + def leasesBaseUri: String + + def authBaseUri: String + + def allInternalUris: Seq[String] + + def guardianWitnessBaseUri: String + + def corsAllowedDomains: Set[String] + + def redirectUriParam: String + + def redirectUriPlaceholder: String + + def loginUriTemplate: String + +} + case class ServiceHosts( - kahunaPrefix: String, - apiPrefix: String, - loaderPrefix: String, - projectionPrefix: String, - cropperPrefix: String, - metadataPrefix: String, - imgopsPrefix: String, - usagePrefix: String, - collectionsPrefix: String, - leasesPrefix: String, - authPrefix: String, - thrallPrefix: String -) + kahunaPrefix: String, + apiPrefix: String, + loaderPrefix: String, + projectionPrefix: String, + cropperPrefix: String, + metadataPrefix: String, + imgopsPrefix: String, + usagePrefix: String, + collectionsPrefix: String, + leasesPrefix: String, + authPrefix: String, + thrallPrefix: String + ) object ServiceHosts { // this is tightly coupled to the Guardian's deployment. @@ -38,32 +76,32 @@ object ServiceHosts { } } -class Services(val domainRoot: String, hosts: ServiceHosts, corsAllowedOrigins: Set[String], domainRootOverride: Option[String] = None) { - private val kahunaHost: String = s"${hosts.kahunaPrefix}$domainRoot" - private val apiHost: String = s"${hosts.apiPrefix}$domainRoot" - private val loaderHost: String = s"${hosts.loaderPrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val cropperHost: String = s"${hosts.cropperPrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val metadataHost: String = s"${hosts.metadataPrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val imgopsHost: String = s"${hosts.imgopsPrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val usageHost: String = s"${hosts.usagePrefix}${domainRootOverride.getOrElse(domainRoot)}" +class GuardianUrlSchemeServices(val domainRoot: String, hosts: ServiceHosts, corsAllowedOrigins: Set[String], domainRootOverride: Option[String] = None) extends Services { + private val kahunaHost: String = s"${hosts.kahunaPrefix}$domainRoot" + private val apiHost: String = s"${hosts.apiPrefix}$domainRoot" + private val loaderHost: String = s"${hosts.loaderPrefix}${domainRootOverride.getOrElse(domainRoot)}" + private val cropperHost: String = s"${hosts.cropperPrefix}${domainRootOverride.getOrElse(domainRoot)}" + private val metadataHost: String = s"${hosts.metadataPrefix}${domainRootOverride.getOrElse(domainRoot)}" + private val imgopsHost: String = s"${hosts.imgopsPrefix}${domainRootOverride.getOrElse(domainRoot)}" + private val usageHost: String = s"${hosts.usagePrefix}${domainRootOverride.getOrElse(domainRoot)}" private val collectionsHost: String = s"${hosts.collectionsPrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val leasesHost: String = s"${hosts.leasesPrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val authHost: String = s"${hosts.authPrefix}$domainRoot" - private val projectionHost: String = s"${hosts.projectionPrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val thrallHost: String = s"${hosts.thrallPrefix}${domainRootOverride.getOrElse(domainRoot)}" - - val kahunaBaseUri = baseUri(kahunaHost) - val apiBaseUri = baseUri(apiHost) - val loaderBaseUri = baseUri(loaderHost) - val projectionBaseUri = baseUri(projectionHost) - val cropperBaseUri = baseUri(cropperHost) - val metadataBaseUri = baseUri(metadataHost) - val imgopsBaseUri = baseUri(imgopsHost) - val usageBaseUri = baseUri(usageHost) + private val leasesHost: String = s"${hosts.leasesPrefix}${domainRootOverride.getOrElse(domainRoot)}" + private val authHost: String = s"${hosts.authPrefix}$domainRoot" + private val projectionHost: String = s"${hosts.projectionPrefix}${domainRootOverride.getOrElse(domainRoot)}" + private val thrallHost: String = s"${hosts.thrallPrefix}${domainRootOverride.getOrElse(domainRoot)}" + + val kahunaBaseUri = baseUri(kahunaHost) + val apiBaseUri = baseUri(apiHost) + val loaderBaseUri = baseUri(loaderHost) + val projectionBaseUri = baseUri(projectionHost) + val cropperBaseUri = baseUri(cropperHost) + val metadataBaseUri = baseUri(metadataHost) + val imgopsBaseUri = baseUri(imgopsHost) + val usageBaseUri = baseUri(usageHost) val collectionsBaseUri = baseUri(collectionsHost) - val leasesBaseUri = baseUri(leasesHost) - val authBaseUri = baseUri(authHost) - val thrallBaseUri = baseUri(thrallHost) + val leasesBaseUri = baseUri(leasesHost) + val authBaseUri = baseUri(authHost) + val thrallBaseUri = baseUri(thrallHost) val allInternalUris = Seq( kahunaBaseUri, @@ -86,5 +124,5 @@ class Services(val domainRoot: String, hosts: ServiceHosts, corsAllowedOrigins: val redirectUriPlaceholder = s"{?$redirectUriParam}" val loginUriTemplate = s"$authBaseUri/login$redirectUriPlaceholder" - def baseUri(host: String) = s"https://$host" + private def baseUri(host: String) = s"https://$host" } diff --git a/image-loader/app/ImageLoaderComponents.scala b/image-loader/app/ImageLoaderComponents.scala index ebe3ef69ee..52c0d09eb9 100644 --- a/image-loader/app/ImageLoaderComponents.scala +++ b/image-loader/app/ImageLoaderComponents.scala @@ -1,6 +1,6 @@ import com.gu.mediaservice.GridClient import com.gu.mediaservice.lib.aws.SimpleSqsMessageConsumer -import com.gu.mediaservice.lib.config.Services +import com.gu.mediaservice.lib.config.{GuardianUrlSchemeServices} import com.gu.mediaservice.lib.imaging.ImageOperations import com.gu.mediaservice.lib.logging.GridLogging import com.gu.mediaservice.lib.management.InnerServiceStatusCheckController @@ -38,7 +38,7 @@ class ImageLoaderComponents(context: Context) extends GridComponents(context, ne case (false, _) => None } - val services = new Services(config.domainRoot, config.serviceHosts, Set.empty) + val services = new GuardianUrlSchemeServices(config.domainRoot, config.serviceHosts, Set.empty) private val gridClient = GridClient(services)(wsClient) val metrics = new ImageLoaderMetrics(config, actorSystem, applicationLifecycle) diff --git a/media-api/app/controllers/MediaApi.scala b/media-api/app/controllers/MediaApi.scala index cb68f98e8f..83a343eafc 100644 --- a/media-api/app/controllers/MediaApi.scala +++ b/media-api/app/controllers/MediaApi.scala @@ -9,7 +9,7 @@ import com.gu.mediaservice.lib.auth.Authentication.{Request, _} import com.gu.mediaservice.lib.auth.Permissions.{ArchiveImages, DeleteCropsOrUsages, EditMetadata, UploadImages, DeleteImage => DeleteImagePermission} import com.gu.mediaservice.lib.auth._ import com.gu.mediaservice.lib.aws.{ContentDisposition, ThrallMessageSender, UpdateMessage} -import com.gu.mediaservice.lib.config.Services +import com.gu.mediaservice.lib.config.{GuardianUrlSchemeServices, Services} import com.gu.mediaservice.lib.formatting.printDateTime import com.gu.mediaservice.lib.logging.MarkerMap import com.gu.mediaservice.lib.metadata.SoftDeletedMetadataTable @@ -44,7 +44,7 @@ class MediaApi( authorisation: Authorisation )(implicit val ec: ExecutionContext) extends BaseController with MessageSubjects with ArgoHelpers with ContentDisposition { - val services: Services = new Services(config.domainRoot, config.serviceHosts, Set.empty) + val services: Services = new GuardianUrlSchemeServices(config.domainRoot, config.serviceHosts, Set.empty) val gridClient: GridClient = GridClient(services)(ws) private val searchParamList = List( diff --git a/metadata-editor/app/controllers/EditsController.scala b/metadata-editor/app/controllers/EditsController.scala index 6ee3079fad..6a20b3e187 100644 --- a/metadata-editor/app/controllers/EditsController.scala +++ b/metadata-editor/app/controllers/EditsController.scala @@ -3,7 +3,6 @@ package controllers import java.net.URI import java.net.URLDecoder.decode - import com.amazonaws.AmazonServiceException import com.gu.mediaservice.GridClient import com.gu.mediaservice.lib.argo.ArgoHelpers @@ -13,7 +12,7 @@ import com.gu.mediaservice.lib.auth.Authentication.Principal import com.gu.mediaservice.lib.auth.Permissions.EditMetadata import com.gu.mediaservice.lib.auth.{Authentication, Authorisation} import com.gu.mediaservice.lib.aws.NoItemFound -import com.gu.mediaservice.lib.config.{ServiceHosts, Services} +import com.gu.mediaservice.lib.config.{GuardianUrlSchemeServices, Services} import com.gu.mediaservice.model._ import com.gu.mediaservice.syntax.MessageSubjects import lib._ @@ -58,7 +57,7 @@ class EditsController( import com.gu.mediaservice.lib.metadata.UsageRightsMetadataMapper.usageRightsToMetadata - val services: Services = new Services(config.domainRoot, config.serviceHosts, Set.empty) + val services: Services = new GuardianUrlSchemeServices(config.domainRoot, config.serviceHosts, Set.empty) val gridClient: GridClient = GridClient(services)(ws) val metadataBaseUri = config.services.metadataBaseUri diff --git a/thrall/app/ThrallComponents.scala b/thrall/app/ThrallComponents.scala index 80b8071218..c0db66b796 100644 --- a/thrall/app/ThrallComponents.scala +++ b/thrall/app/ThrallComponents.scala @@ -2,7 +2,7 @@ import org.apache.pekko.Done import org.apache.pekko.stream.scaladsl.Source import com.gu.kinesis.{KinesisRecord, KinesisSource, ConsumerConfig => KclPekkoStreamConfig} import com.gu.mediaservice.GridClient -import com.gu.mediaservice.lib.config.Services +import com.gu.mediaservice.lib.config.{GuardianUrlSchemeServices, Services} import com.gu.mediaservice.lib.aws.{S3Ops, ThrallMessageSender} import com.gu.mediaservice.lib.management.InnerServiceStatusCheckController import com.gu.mediaservice.lib.metadata.SoftDeletedMetadataTable @@ -30,7 +30,7 @@ class ThrallComponents(context: Context) extends GridComponents(context, new Thr val es = new ElasticSearch(config.esConfig, Some(thrallMetrics), actorSystem.scheduler) es.ensureIndexExistsAndAliasAssigned() - val services: Services = new Services(config.domainRoot, config.serviceHosts, Set.empty) + val services: Services = new GuardianUrlSchemeServices(config.domainRoot, config.serviceHosts, Set.empty) val gridClient: GridClient = GridClient(services)(wsClient) // before firing up anything to consume streams or say we are OK let's do the critical good to go check From 130a7819a98b076bf09c856cec42dd135d3d4416 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Wed, 8 May 2024 11:38:56 +0100 Subject: [PATCH 14/27] Everyone using GuardianUrlSchemeServices directly should take Services supplied by common config; no need to trouble yourselves with the details of how those urls are defined. --- image-loader/app/ImageLoaderComponents.scala | 4 +--- media-api/app/controllers/MediaApi.scala | 4 +--- metadata-editor/app/controllers/EditsController.scala | 4 +--- thrall/app/ThrallComponents.scala | 4 +--- 4 files changed, 4 insertions(+), 12 deletions(-) diff --git a/image-loader/app/ImageLoaderComponents.scala b/image-loader/app/ImageLoaderComponents.scala index 52c0d09eb9..bde7b0849e 100644 --- a/image-loader/app/ImageLoaderComponents.scala +++ b/image-loader/app/ImageLoaderComponents.scala @@ -1,6 +1,5 @@ import com.gu.mediaservice.GridClient import com.gu.mediaservice.lib.aws.SimpleSqsMessageConsumer -import com.gu.mediaservice.lib.config.{GuardianUrlSchemeServices} import com.gu.mediaservice.lib.imaging.ImageOperations import com.gu.mediaservice.lib.logging.GridLogging import com.gu.mediaservice.lib.management.InnerServiceStatusCheckController @@ -38,8 +37,7 @@ class ImageLoaderComponents(context: Context) extends GridComponents(context, ne case (false, _) => None } - val services = new GuardianUrlSchemeServices(config.domainRoot, config.serviceHosts, Set.empty) - private val gridClient = GridClient(services)(wsClient) + private val gridClient = GridClient(config.services)(wsClient) val metrics = new ImageLoaderMetrics(config, actorSystem, applicationLifecycle) diff --git a/media-api/app/controllers/MediaApi.scala b/media-api/app/controllers/MediaApi.scala index 83a343eafc..d588221531 100644 --- a/media-api/app/controllers/MediaApi.scala +++ b/media-api/app/controllers/MediaApi.scala @@ -9,7 +9,6 @@ import com.gu.mediaservice.lib.auth.Authentication.{Request, _} import com.gu.mediaservice.lib.auth.Permissions.{ArchiveImages, DeleteCropsOrUsages, EditMetadata, UploadImages, DeleteImage => DeleteImagePermission} import com.gu.mediaservice.lib.auth._ import com.gu.mediaservice.lib.aws.{ContentDisposition, ThrallMessageSender, UpdateMessage} -import com.gu.mediaservice.lib.config.{GuardianUrlSchemeServices, Services} import com.gu.mediaservice.lib.formatting.printDateTime import com.gu.mediaservice.lib.logging.MarkerMap import com.gu.mediaservice.lib.metadata.SoftDeletedMetadataTable @@ -44,8 +43,7 @@ class MediaApi( authorisation: Authorisation )(implicit val ec: ExecutionContext) extends BaseController with MessageSubjects with ArgoHelpers with ContentDisposition { - val services: Services = new GuardianUrlSchemeServices(config.domainRoot, config.serviceHosts, Set.empty) - val gridClient: GridClient = GridClient(services)(ws) + private val gridClient: GridClient = GridClient(config.services)(ws) private val searchParamList = List( "q", diff --git a/metadata-editor/app/controllers/EditsController.scala b/metadata-editor/app/controllers/EditsController.scala index 6a20b3e187..48df88e16e 100644 --- a/metadata-editor/app/controllers/EditsController.scala +++ b/metadata-editor/app/controllers/EditsController.scala @@ -12,7 +12,6 @@ import com.gu.mediaservice.lib.auth.Authentication.Principal import com.gu.mediaservice.lib.auth.Permissions.EditMetadata import com.gu.mediaservice.lib.auth.{Authentication, Authorisation} import com.gu.mediaservice.lib.aws.NoItemFound -import com.gu.mediaservice.lib.config.{GuardianUrlSchemeServices, Services} import com.gu.mediaservice.model._ import com.gu.mediaservice.syntax.MessageSubjects import lib._ @@ -57,8 +56,7 @@ class EditsController( import com.gu.mediaservice.lib.metadata.UsageRightsMetadataMapper.usageRightsToMetadata - val services: Services = new GuardianUrlSchemeServices(config.domainRoot, config.serviceHosts, Set.empty) - val gridClient: GridClient = GridClient(services)(ws) + private val gridClient: GridClient = GridClient(config.services)(ws) val metadataBaseUri = config.services.metadataBaseUri private val AuthenticatedAndAuthorised = auth andThen authorisation.CommonActionFilters.authorisedForArchive diff --git a/thrall/app/ThrallComponents.scala b/thrall/app/ThrallComponents.scala index c0db66b796..8ef49988fb 100644 --- a/thrall/app/ThrallComponents.scala +++ b/thrall/app/ThrallComponents.scala @@ -2,7 +2,6 @@ import org.apache.pekko.Done import org.apache.pekko.stream.scaladsl.Source import com.gu.kinesis.{KinesisRecord, KinesisSource, ConsumerConfig => KclPekkoStreamConfig} import com.gu.mediaservice.GridClient -import com.gu.mediaservice.lib.config.{GuardianUrlSchemeServices, Services} import com.gu.mediaservice.lib.aws.{S3Ops, ThrallMessageSender} import com.gu.mediaservice.lib.management.InnerServiceStatusCheckController import com.gu.mediaservice.lib.metadata.SoftDeletedMetadataTable @@ -30,8 +29,7 @@ class ThrallComponents(context: Context) extends GridComponents(context, new Thr val es = new ElasticSearch(config.esConfig, Some(thrallMetrics), actorSystem.scheduler) es.ensureIndexExistsAndAliasAssigned() - val services: Services = new GuardianUrlSchemeServices(config.domainRoot, config.serviceHosts, Set.empty) - val gridClient: GridClient = GridClient(services)(wsClient) + val gridClient: GridClient = GridClient(config.services)(wsClient) // before firing up anything to consume streams or say we are OK let's do the critical good to go check private val goodToGoCheckResult = Await.ready(GoodToGoCheck.run(es), 30 seconds) From 190c4640afbe99651298d6b8dae24d348b25eadc Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Wed, 8 May 2024 12:14:25 +0100 Subject: [PATCH 15/27] Drop GuardianUrlSchemeServices val constructor fields; these allow undocumented access to the private url building concerns. --- .../main/scala/com/gu/mediaservice/lib/config/Services.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala index 3c7d203167..0e2fa014fe 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala @@ -76,7 +76,7 @@ object ServiceHosts { } } -class GuardianUrlSchemeServices(val domainRoot: String, hosts: ServiceHosts, corsAllowedOrigins: Set[String], domainRootOverride: Option[String] = None) extends Services { +class GuardianUrlSchemeServices(domainRoot: String, hosts: ServiceHosts, corsAllowedOrigins: Set[String], domainRootOverride: Option[String] = None) extends Services { private val kahunaHost: String = s"${hosts.kahunaPrefix}$domainRoot" private val apiHost: String = s"${hosts.apiPrefix}$domainRoot" private val loaderHost: String = s"${hosts.loaderPrefix}${domainRootOverride.getOrElse(domainRoot)}" From bb051d42f48bd8502bba3de8f3c054f1e0e75cc8 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Wed, 8 May 2024 12:02:16 +0100 Subject: [PATCH 16/27] Add a Service URL implementation which map services to port numbers on the single hostname. Will work because HTTPS auth is not active. CORS for single host urls. Projection end points are on the image-loader service but have seperate config to permit reingession workloads to be on different instances. --- .../lib/config/CommonConfig.scala | 2 +- .../gu/mediaservice/lib/config/Services.scala | 53 ++++++++++++++++++- .../lib/play/GridComponents.scala | 4 +- 3 files changed, 55 insertions(+), 4 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 621fc7016a..ab5592c977 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 @@ -77,7 +77,7 @@ abstract class CommonConfig(resources: GridConfigResources) extends AwsClientV1B val corsAllowedOrigins: Set[String] = getStringSet("security.cors.allowedOrigins") - val services = new GuardianUrlSchemeServices(domainRoot, serviceHosts, corsAllowedOrigins, domainRootOverride) + val services = new SingleHostServices("york.local", 32400) /** * Load in a list of domain metadata specifications from configuration. For example: diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala index 0e2fa014fe..9a93d35c74 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala @@ -76,7 +76,58 @@ object ServiceHosts { } } -class GuardianUrlSchemeServices(domainRoot: String, hosts: ServiceHosts, corsAllowedOrigins: Set[String], domainRootOverride: Option[String] = None) extends Services { +protected class SingleHostServices(val hostname: String, baseport: Int) extends Services { + val kahunaBaseUri: String = baseUri(hostname, baseport + 20) + + val apiBaseUri: String = baseUri(hostname, baseport + 1) + + val loaderBaseUri: String = baseUri(hostname, baseport + 3) + + val projectionBaseUri: String = loaderBaseUri + + val cropperBaseUri: String = baseUri(hostname, baseport + 6) + + val metadataBaseUri: String = baseUri(hostname, baseport + 7) + + val imgopsBaseUri: String = baseUri(hostname, baseport + 8) + + val usageBaseUri: String = baseUri(hostname, baseport + 9) + + val collectionsBaseUri: String = baseUri(hostname, baseport + 10) + + val leasesBaseUri: String = baseUri(hostname, baseport + 12) + + val authBaseUri: String = baseUri(hostname, baseport + 11) + + private val thrallBaseUri: String = baseUri(hostname, baseport + 200) + + val allInternalUris: Seq[String] = Seq( + kahunaBaseUri, + apiBaseUri, + loaderBaseUri, + cropperBaseUri, + metadataBaseUri, + usageBaseUri, + collectionsBaseUri, + leasesBaseUri, + authBaseUri, + thrallBaseUri + ) + + val guardianWitnessBaseUri: String = "https://n0ticeapis.com" + + val corsAllowedDomains: Set[String] = Set(kahunaBaseUri, apiBaseUri, thrallBaseUri) + + val redirectUriParam = "redirectUri" + val redirectUriPlaceholder = s"{?$redirectUriParam}" + val loginUriTemplate = s"$authBaseUri/login$redirectUriPlaceholder" + + + private def baseUri(host: String, port: Int) = s"http://$host:$port" + +} + +protected class GuardianUrlSchemeServices(domainRoot: String, hosts: ServiceHosts, corsAllowedOrigins: Set[String], domainRootOverride: Option[String] = None) extends Services { private val kahunaHost: String = s"${hosts.kahunaPrefix}$domainRoot" private val apiHost: String = s"${hosts.apiPrefix}$domainRoot" private val loaderHost: String = s"${hosts.loaderPrefix}${domainRootOverride.getOrElse(domainRoot)}" diff --git a/rest-lib/src/main/scala/com/gu/mediaservice/lib/play/GridComponents.scala b/rest-lib/src/main/scala/com/gu/mediaservice/lib/play/GridComponents.scala index 354613905d..fd9aca2691 100644 --- a/rest-lib/src/main/scala/com/gu/mediaservice/lib/play/GridComponents.scala +++ b/rest-lib/src/main/scala/com/gu/mediaservice/lib/play/GridComponents.scala @@ -39,8 +39,8 @@ abstract class GridComponents[Config <: CommonConfig](context: Context, val load ) final override lazy val corsConfig: CORSConfig = CORSConfig.fromConfiguration(context.initialConfiguration).copy( - allowedOrigins = Origins.Matching(config.services.corsAllowedDomains) - ) + allowedOrigins = Origins.Matching(config.services.corsAllowedDomains) + ) lazy val management = new Management(controllerComponents, buildInfo) From 22d904f8d08d83a69c55c65e6979102374830f32 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Mon, 3 Jun 2024 14:50:15 +0100 Subject: [PATCH 17/27] Drop Guardian services URL scheme. We have used it to shape the Service interface. It can be dropped now. --- .../lib/config/CommonConfig.scala | 14 --- .../gu/mediaservice/lib/config/Services.scala | 90 +------------------ 2 files changed, 1 insertion(+), 103 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 ab5592c977..222c26ef35 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 @@ -60,20 +60,6 @@ abstract class CommonConfig(resources: GridConfigResources) extends AwsClientV1B val domainRoot: String = string("domain.root") val domainRootOverride: Option[String] = stringOpt("domain.root-override") val rootAppName: String = stringDefault("app.name.root", "media") - val serviceHosts = ServiceHosts( - stringDefault("hosts.kahunaPrefix", s"$rootAppName."), - stringDefault("hosts.apiPrefix", s"api.$rootAppName."), - stringDefault("hosts.loaderPrefix", s"loader.$rootAppName."), - stringDefault("hosts.projectionPrefix", s"loader-projection.$rootAppName."), - stringDefault("hosts.cropperPrefix", s"cropper.$rootAppName."), - stringDefault("hosts.metadataPrefix", s"$rootAppName-metadata."), - stringDefault("hosts.imgopsPrefix", s"$rootAppName-imgops."), - stringDefault("hosts.usagePrefix", s"$rootAppName-usage."), - stringDefault("hosts.collectionsPrefix", s"$rootAppName-collections."), - stringDefault("hosts.leasesPrefix", s"$rootAppName-leases."), - stringDefault("hosts.authPrefix", s"$rootAppName-auth."), - stringDefault("hosts.thrallPrefix", s"thrall.$rootAppName.") - ) val corsAllowedOrigins: Set[String] = getStringSet("security.cors.allowedOrigins") diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala index 9a93d35c74..b67802a64b 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala @@ -38,45 +38,7 @@ trait Services { } -case class ServiceHosts( - kahunaPrefix: String, - apiPrefix: String, - loaderPrefix: String, - projectionPrefix: String, - cropperPrefix: String, - metadataPrefix: String, - imgopsPrefix: String, - usagePrefix: String, - collectionsPrefix: String, - leasesPrefix: String, - authPrefix: String, - thrallPrefix: String - ) - -object ServiceHosts { - // this is tightly coupled to the Guardian's deployment. - // TODO make more generic but w/out relying on Play config - def guardianPrefixes: ServiceHosts = { - val rootAppName: String = "media" - - ServiceHosts( - kahunaPrefix = s"$rootAppName.", - apiPrefix = s"api.$rootAppName.", - loaderPrefix = s"loader.$rootAppName.", - projectionPrefix = s"loader-projection.$rootAppName", - cropperPrefix = s"cropper.$rootAppName.", - metadataPrefix = s"$rootAppName-metadata.", - imgopsPrefix = s"$rootAppName-imgops.", - usagePrefix = s"$rootAppName-usage.", - collectionsPrefix = s"$rootAppName-collections.", - leasesPrefix = s"$rootAppName-leases.", - authPrefix = s"$rootAppName-auth.", - thrallPrefix = s"thrall.$rootAppName." - ) - } -} - -protected class SingleHostServices(val hostname: String, baseport: Int) extends Services { +protected class SingleHostServices(val hostname: String, val baseport: Int) extends Services { val kahunaBaseUri: String = baseUri(hostname, baseport + 20) val apiBaseUri: String = baseUri(hostname, baseport + 1) @@ -127,53 +89,3 @@ protected class SingleHostServices(val hostname: String, baseport: Int) extends } -protected class GuardianUrlSchemeServices(domainRoot: String, hosts: ServiceHosts, corsAllowedOrigins: Set[String], domainRootOverride: Option[String] = None) extends Services { - private val kahunaHost: String = s"${hosts.kahunaPrefix}$domainRoot" - private val apiHost: String = s"${hosts.apiPrefix}$domainRoot" - private val loaderHost: String = s"${hosts.loaderPrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val cropperHost: String = s"${hosts.cropperPrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val metadataHost: String = s"${hosts.metadataPrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val imgopsHost: String = s"${hosts.imgopsPrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val usageHost: String = s"${hosts.usagePrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val collectionsHost: String = s"${hosts.collectionsPrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val leasesHost: String = s"${hosts.leasesPrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val authHost: String = s"${hosts.authPrefix}$domainRoot" - private val projectionHost: String = s"${hosts.projectionPrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val thrallHost: String = s"${hosts.thrallPrefix}${domainRootOverride.getOrElse(domainRoot)}" - - val kahunaBaseUri = baseUri(kahunaHost) - val apiBaseUri = baseUri(apiHost) - val loaderBaseUri = baseUri(loaderHost) - val projectionBaseUri = baseUri(projectionHost) - val cropperBaseUri = baseUri(cropperHost) - val metadataBaseUri = baseUri(metadataHost) - val imgopsBaseUri = baseUri(imgopsHost) - val usageBaseUri = baseUri(usageHost) - val collectionsBaseUri = baseUri(collectionsHost) - val leasesBaseUri = baseUri(leasesHost) - val authBaseUri = baseUri(authHost) - val thrallBaseUri = baseUri(thrallHost) - - val allInternalUris = Seq( - kahunaBaseUri, - apiBaseUri, - loaderBaseUri, - cropperBaseUri, - metadataBaseUri, - usageBaseUri, - collectionsBaseUri, - leasesBaseUri, - authBaseUri, - thrallBaseUri - ) - - val guardianWitnessBaseUri: String = "https://n0ticeapis.com" - - val corsAllowedDomains: Set[String] = corsAllowedOrigins.map(baseUri) + kahunaBaseUri + apiBaseUri + thrallBaseUri - - val redirectUriParam = "redirectUri" - val redirectUriPlaceholder = s"{?$redirectUriParam}" - val loginUriTemplate = s"$authBaseUri/login$redirectUriPlaceholder" - - private def baseUri(host: String) = s"https://$host" -} From a20c7647a87849831d13560bed3fa2c09ab74537 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Fri, 10 May 2024 16:07:30 +0100 Subject: [PATCH 18/27] Move all public facing service urls to sub paths under single hostname. Config single.host.url is exclusively for our single host setup. --- .../lib/config/CommonConfig.scala | 3 +- .../gu/mediaservice/lib/config/Services.scala | 28 +++++++++---------- .../src/test/resources/application.conf | 1 + .../test/lib/elasticsearch/Fixtures.scala | 1 + 4 files changed, 17 insertions(+), 16 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 222c26ef35..0eb89d4bd4 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 @@ -63,7 +63,8 @@ abstract class CommonConfig(resources: GridConfigResources) extends AwsClientV1B val corsAllowedOrigins: Set[String] = getStringSet("security.cors.allowedOrigins") - val services = new SingleHostServices("york.local", 32400) + private val singleHostUrl: String = string("single.host.url") + val services = new SingleHostServices(singleHostUrl) /** * Load in a list of domain metadata specifications from configuration. For example: diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala index b67802a64b..83c55a66b2 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala @@ -38,30 +38,30 @@ trait Services { } -protected class SingleHostServices(val hostname: String, val baseport: Int) extends Services { - val kahunaBaseUri: String = baseUri(hostname, baseport + 20) +protected class SingleHostServices(val rootUrl: String) extends Services { + val kahunaBaseUri: String = rootUrl - val apiBaseUri: String = baseUri(hostname, baseport + 1) + val apiBaseUri: String = subpathedServiceBaseUri("media-api") - val loaderBaseUri: String = baseUri(hostname, baseport + 3) + val loaderBaseUri: String = subpathedServiceBaseUri("image-loader") val projectionBaseUri: String = loaderBaseUri - val cropperBaseUri: String = baseUri(hostname, baseport + 6) + val cropperBaseUri: String = subpathedServiceBaseUri("cropper") - val metadataBaseUri: String = baseUri(hostname, baseport + 7) + val metadataBaseUri: String = subpathedServiceBaseUri("metadata-editor") - val imgopsBaseUri: String = baseUri(hostname, baseport + 8) + val imgopsBaseUri: String = subpathedServiceBaseUri("imgops") - val usageBaseUri: String = baseUri(hostname, baseport + 9) + val usageBaseUri: String =subpathedServiceBaseUri("usage") - val collectionsBaseUri: String = baseUri(hostname, baseport + 10) + val collectionsBaseUri: String = subpathedServiceBaseUri("collections") - val leasesBaseUri: String = baseUri(hostname, baseport + 12) + val leasesBaseUri: String = subpathedServiceBaseUri("leases") - val authBaseUri: String = baseUri(hostname, baseport + 11) + val authBaseUri: String = subpathedServiceBaseUri("auth") - private val thrallBaseUri: String = baseUri(hostname, baseport + 200) + private val thrallBaseUri: String = subpathedServiceBaseUri("thrall") val allInternalUris: Seq[String] = Seq( kahunaBaseUri, @@ -84,8 +84,6 @@ protected class SingleHostServices(val hostname: String, val baseport: Int) exte val redirectUriPlaceholder = s"{?$redirectUriParam}" val loginUriTemplate = s"$authBaseUri/login$redirectUriPlaceholder" - - private def baseUri(host: String, port: Int) = s"http://$host:$port" - + private def subpathedServiceBaseUri(serviceName: String): String = s"$rootUrl/$serviceName" } diff --git a/common-lib/src/test/resources/application.conf b/common-lib/src/test/resources/application.conf index 962a767889..fa78ccd921 100644 --- a/common-lib/src/test/resources/application.conf +++ b/common-lib/src/test/resources/application.conf @@ -3,6 +3,7 @@ grid.appName: "test" thrall.kinesis.stream.name: "not-used" thrall.kinesis.lowPriorityStream.name: "not-used" domain.root: "notused.example.com" +single.host.url: "notused.example.com" image.processors = [ "com.gu.mediaservice.lib.cleanup.GuardianMetadataCleaners", diff --git a/media-api/test/lib/elasticsearch/Fixtures.scala b/media-api/test/lib/elasticsearch/Fixtures.scala index 50666174cc..3b2876a679 100644 --- a/media-api/test/lib/elasticsearch/Fixtures.scala +++ b/media-api/test/lib/elasticsearch/Fixtures.scala @@ -30,6 +30,7 @@ trait Fixtures { "thrall.kinesis.stream.name", "thrall.kinesis.lowPriorityStream.name", "domain.root", + "single.host.url", "s3.config.bucket", "s3.usagemail.bucket", "quota.store.key", From 8daf27772fdaee3995d7d5e69e5d30b35fbc211b Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Tue, 14 May 2024 08:26:35 +0100 Subject: [PATCH 19/27] Disable CSRF with is no longer bypassed on a single origin CORS check. No longer gets bypassed thanks to preceding CORS check; CORS filter does not appear to tag the request if it passes for same origin. --- .../scala/com/gu/mediaservice/lib/play/GridComponents.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest-lib/src/main/scala/com/gu/mediaservice/lib/play/GridComponents.scala b/rest-lib/src/main/scala/com/gu/mediaservice/lib/play/GridComponents.scala index fd9aca2691..e272581a20 100644 --- a/rest-lib/src/main/scala/com/gu/mediaservice/lib/play/GridComponents.scala +++ b/rest-lib/src/main/scala/com/gu/mediaservice/lib/play/GridComponents.scala @@ -30,7 +30,7 @@ abstract class GridComponents[Config <: CommonConfig](context: Context, val load final override def httpFilters: Seq[EssentialFilter] = Seq( corsFilter, - csrfFilter, + //csrfFilter TODO no longer gets bypassed thanks to preceding CORS check; CORS filter does not appear to tag the request if it passes for same origin. securityHeadersFilter, gzipFilter, new RequestLoggingFilter(materializer), From 60e2a19adcb9ba17d229aa705bd8295159860ed4 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Tue, 14 May 2024 17:54:26 +0100 Subject: [PATCH 20/27] Delete InnerServiceStatusCheckController "checks connectivity to all other internal services..." which sounds like something we can let the container orchestrator handle. InnerServiceStatusCheckController was the only user of Services.allInternalUris --- auth/app/auth/AuthComponents.scala | 5 +- auth/conf/routes | 1 - collections/app/CollectionsComponents.scala | 4 +- collections/conf/routes | 1 - .../gu/mediaservice/lib/config/Services.scala | 15 ----- cropper/app/CropperComponents.scala | 5 +- cropper/conf/routes | 1 - image-loader/app/ImageLoaderComponents.scala | 4 +- image-loader/conf/routes | 1 - kahuna/app/KahunaComponents.scala | 4 +- kahuna/conf/routes | 1 - leases/app/LeasesComponents.scala | 4 +- leases/conf/routes | 1 - media-api/app/MediaApiComponents.scala | 6 +- media-api/conf/routes | 1 - .../app/MetadataEditorComponents.scala | 4 +- metadata-editor/conf/routes | 1 - .../InnerServiceStatusCheckController.scala | 67 ------------------- thrall/app/ThrallComponents.scala | 4 +- thrall/conf/routes | 2 - usage/app/UsageComponents.scala | 4 +- usage/conf/routes | 1 - 22 files changed, 13 insertions(+), 124 deletions(-) diff --git a/auth/app/auth/AuthComponents.scala b/auth/app/auth/AuthComponents.scala index 1077bb8118..0882a1086a 100644 --- a/auth/app/auth/AuthComponents.scala +++ b/auth/app/auth/AuthComponents.scala @@ -1,6 +1,6 @@ package auth -import com.gu.mediaservice.lib.management.{InnerServiceStatusCheckController, Management} +import com.gu.mediaservice.lib.management.Management import com.gu.mediaservice.lib.play.GridComponents import play.api.ApplicationLoader.Context import play.api.{Configuration, Environment} @@ -14,10 +14,9 @@ class AuthComponents(context: Context) extends GridComponents(context, new AuthC val controller = new AuthController(auth, providers, config, controllerComponents, authorisation) val permissionsAwareManagement = new Management(controllerComponents, buildInfo) - val InnerServiceStatusCheckController = new InnerServiceStatusCheckController(auth, controllerComponents, config.services, wsClient) - override val router = new Routes(httpErrorHandler, controller, permissionsAwareManagement, InnerServiceStatusCheckController) + override val router = new Routes(httpErrorHandler, controller, permissionsAwareManagement) } object AuthHttpConfig { diff --git a/auth/conf/routes b/auth/conf/routes index 9a58260935..4423cc15e9 100644 --- a/auth/conf/routes +++ b/auth/conf/routes @@ -16,7 +16,6 @@ GET /cookieMonster auth.AuthController.cookieMonster # Management GET /management/healthcheck com.gu.mediaservice.lib.management.Management.healthCheck GET /management/manifest com.gu.mediaservice.lib.management.Management.manifest -GET /management/whoAmI com.gu.mediaservice.lib.management.InnerServiceStatusCheckController.whoAmI(depth: Int) # Shoo robots away GET /robots.txt com.gu.mediaservice.lib.management.Management.disallowRobots diff --git a/collections/app/CollectionsComponents.scala b/collections/app/CollectionsComponents.scala index 046c99e065..4038acef03 100644 --- a/collections/app/CollectionsComponents.scala +++ b/collections/app/CollectionsComponents.scala @@ -1,4 +1,3 @@ -import com.gu.mediaservice.lib.management.InnerServiceStatusCheckController import com.gu.mediaservice.lib.play.GridComponents import controllers.{CollectionsController, ImageCollectionsController} import lib.{CollectionsConfig, CollectionsMetrics, Notifications} @@ -15,8 +14,7 @@ class CollectionsComponents(context: Context) extends GridComponents(context, ne val collections = new CollectionsController(auth, config, store, controllerComponents) val imageCollections = new ImageCollectionsController(auth, config, notifications, controllerComponents) - val InnerServiceStatusCheckController = new InnerServiceStatusCheckController(auth, controllerComponents, config.services, wsClient) - override val router = new Routes(httpErrorHandler, collections, imageCollections, management, InnerServiceStatusCheckController) + override val router = new Routes(httpErrorHandler, collections, imageCollections, management) } diff --git a/collections/conf/routes b/collections/conf/routes index 21acc8e124..8fd9d4424d 100644 --- a/collections/conf/routes +++ b/collections/conf/routes @@ -16,7 +16,6 @@ POST /corrected-collections controllers.CollectionsC # Management GET /management/healthcheck com.gu.mediaservice.lib.management.Management.healthCheck GET /management/manifest com.gu.mediaservice.lib.management.Management.manifest -GET /management/whoAmI com.gu.mediaservice.lib.management.InnerServiceStatusCheckController.whoAmI(depth: Int) # Shoo robots away GET /robots.txt com.gu.mediaservice.lib.management.Management.disallowRobots diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala index 83c55a66b2..dc031c0641 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala @@ -24,8 +24,6 @@ trait Services { def authBaseUri: String - def allInternalUris: Seq[String] - def guardianWitnessBaseUri: String def corsAllowedDomains: Set[String] @@ -63,19 +61,6 @@ protected class SingleHostServices(val rootUrl: String) extends Services { private val thrallBaseUri: String = subpathedServiceBaseUri("thrall") - val allInternalUris: Seq[String] = Seq( - kahunaBaseUri, - apiBaseUri, - loaderBaseUri, - cropperBaseUri, - metadataBaseUri, - usageBaseUri, - collectionsBaseUri, - leasesBaseUri, - authBaseUri, - thrallBaseUri - ) - val guardianWitnessBaseUri: String = "https://n0ticeapis.com" val corsAllowedDomains: Set[String] = Set(kahunaBaseUri, apiBaseUri, thrallBaseUri) diff --git a/cropper/app/CropperComponents.scala b/cropper/app/CropperComponents.scala index 3770542919..b3982f6f29 100644 --- a/cropper/app/CropperComponents.scala +++ b/cropper/app/CropperComponents.scala @@ -1,6 +1,6 @@ import com.gu.mediaservice.GridClient import com.gu.mediaservice.lib.imaging.ImageOperations -import com.gu.mediaservice.lib.management.{InnerServiceStatusCheckController, Management} +import com.gu.mediaservice.lib.management.Management import com.gu.mediaservice.lib.play.GridComponents import controllers.CropperController import lib.{CropStore, CropperConfig, Crops, Notifications} @@ -20,8 +20,7 @@ class CropperComponents(context: Context) extends GridComponents(context, new Cr val controller = new CropperController(auth, crops, store, notifications, config, controllerComponents, authorisation, gridClient) val permissionsAwareManagement = new Management(controllerComponents, buildInfo) - val InnerServiceStatusCheckController = new InnerServiceStatusCheckController(auth, controllerComponents, config.services, wsClient) - override lazy val router = new Routes(httpErrorHandler, controller, permissionsAwareManagement, InnerServiceStatusCheckController) + override lazy val router = new Routes(httpErrorHandler, controller, permissionsAwareManagement) } diff --git a/cropper/conf/routes b/cropper/conf/routes index c4aad4e4ee..207c5df4b1 100644 --- a/cropper/conf/routes +++ b/cropper/conf/routes @@ -8,7 +8,6 @@ DELETE /crops/:id controllers.CropperControl # Management GET /management/healthcheck com.gu.mediaservice.lib.management.Management.healthCheck GET /management/manifest com.gu.mediaservice.lib.management.Management.manifest -GET /management/whoAmI com.gu.mediaservice.lib.management.InnerServiceStatusCheckController.whoAmI(depth: Int) # Shoo robots away GET /robots.txt com.gu.mediaservice.lib.management.Management.disallowRobots diff --git a/image-loader/app/ImageLoaderComponents.scala b/image-loader/app/ImageLoaderComponents.scala index bde7b0849e..4d8c8f2d00 100644 --- a/image-loader/app/ImageLoaderComponents.scala +++ b/image-loader/app/ImageLoaderComponents.scala @@ -2,7 +2,6 @@ import com.gu.mediaservice.GridClient import com.gu.mediaservice.lib.aws.SimpleSqsMessageConsumer import com.gu.mediaservice.lib.imaging.ImageOperations import com.gu.mediaservice.lib.logging.GridLogging -import com.gu.mediaservice.lib.management.InnerServiceStatusCheckController import com.gu.mediaservice.lib.play.GridComponents import controllers.{ImageLoaderController, ImageLoaderManagement, UploadStatusController} import lib._ @@ -44,8 +43,7 @@ class ImageLoaderComponents(context: Context) extends GridComponents(context, ne val controller = new ImageLoaderController( auth, downloader, store, maybeIngestQueue, uploadStatusTable, notifications, config, uploader, quarantineUploader, projector, controllerComponents, gridClient, authorisation, metrics) val uploadStatusController = new UploadStatusController(auth, uploadStatusTable, config, controllerComponents, authorisation) - val InnerServiceStatusCheckController = new InnerServiceStatusCheckController(auth, controllerComponents, config.services, wsClient) val imageLoaderManagement = new ImageLoaderManagement(controllerComponents, buildInfo, controller.maybeIngestQueueAndProcessor) - override lazy val router = new Routes(httpErrorHandler, controller, uploadStatusController, imageLoaderManagement, InnerServiceStatusCheckController) + override lazy val router = new Routes(httpErrorHandler, controller, uploadStatusController, imageLoaderManagement) } diff --git a/image-loader/conf/routes b/image-loader/conf/routes index 9987bc648c..7b7b775870 100644 --- a/image-loader/conf/routes +++ b/image-loader/conf/routes @@ -15,7 +15,6 @@ GET /uploadStatuses/:userId controllers.UploadStatusCo # Management GET /management/healthcheck com.gu.mediaservice.lib.management.Management.healthCheck GET /management/manifest com.gu.mediaservice.lib.management.Management.manifest -GET /management/whoAmI com.gu.mediaservice.lib.management.InnerServiceStatusCheckController.whoAmI(depth: Int) # Shoo robots away GET /robots.txt com.gu.mediaservice.lib.management.Management.disallowRobots diff --git a/kahuna/app/KahunaComponents.scala b/kahuna/app/KahunaComponents.scala index 12ad004132..09d93bff08 100644 --- a/kahuna/app/KahunaComponents.scala +++ b/kahuna/app/KahunaComponents.scala @@ -1,4 +1,3 @@ -import com.gu.mediaservice.lib.management.InnerServiceStatusCheckController import com.gu.mediaservice.lib.net.URI import com.gu.mediaservice.lib.play.GridComponents import controllers.{AssetsComponents, KahunaController} @@ -14,9 +13,8 @@ class KahunaComponents(context: Context) extends GridComponents(context, new Kah final override val buildInfo = utils.buildinfo.BuildInfo val controller = new KahunaController(auth, config, controllerComponents, authorisation) - val InnerServiceStatusCheckController = new InnerServiceStatusCheckController(auth, controllerComponents, config.services, wsClient) - final override val router = new Routes(httpErrorHandler, controller, assets, management, InnerServiceStatusCheckController) + final override val router = new Routes(httpErrorHandler, controller, assets, management) } diff --git a/kahuna/conf/routes b/kahuna/conf/routes index b8ad420d3c..3003723ff0 100644 --- a/kahuna/conf/routes +++ b/kahuna/conf/routes @@ -18,7 +18,6 @@ GET /assets/*file controllers.Assets.version # Management GET /management/healthcheck com.gu.mediaservice.lib.management.Management.healthCheck GET /management/manifest com.gu.mediaservice.lib.management.Management.manifest -GET /management/whoAmI com.gu.mediaservice.lib.management.InnerServiceStatusCheckController.whoAmI(depth: Int) # Shoo robots away GET /robots.txt com.gu.mediaservice.lib.management.Management.disallowRobots diff --git a/leases/app/LeasesComponents.scala b/leases/app/LeasesComponents.scala index e236f6f313..c0aa399963 100644 --- a/leases/app/LeasesComponents.scala +++ b/leases/app/LeasesComponents.scala @@ -1,4 +1,3 @@ -import com.gu.mediaservice.lib.management.InnerServiceStatusCheckController import com.gu.mediaservice.lib.play.GridComponents import controllers.MediaLeaseController import lib.{LeaseNotifier, LeaseStore, LeasesConfig} @@ -12,8 +11,7 @@ class LeasesComponents(context: Context) extends GridComponents(context, new Lea val notifications = new LeaseNotifier(config, store) val controller = new MediaLeaseController(auth, store, config, notifications, controllerComponents) - val InnerServiceStatusCheckController = new InnerServiceStatusCheckController(auth, controllerComponents, config.services, wsClient) - override lazy val router = new Routes(httpErrorHandler, controller, management, InnerServiceStatusCheckController) + override lazy val router = new Routes(httpErrorHandler, controller, management) } diff --git a/leases/conf/routes b/leases/conf/routes index f5849a960d..2402837822 100644 --- a/leases/conf/routes +++ b/leases/conf/routes @@ -15,7 +15,6 @@ POST /leases controllers.MediaLeaseCont # Management GET /management/healthcheck com.gu.mediaservice.lib.management.Management.healthCheck GET /management/manifest com.gu.mediaservice.lib.management.Management.manifest -GET /management/whoAmI com.gu.mediaservice.lib.management.InnerServiceStatusCheckController.whoAmI(depth: Int) # Shoo robots away diff --git a/media-api/app/MediaApiComponents.scala b/media-api/app/MediaApiComponents.scala index b77dd908aa..101be58216 100644 --- a/media-api/app/MediaApiComponents.scala +++ b/media-api/app/MediaApiComponents.scala @@ -1,5 +1,5 @@ import com.gu.mediaservice.lib.aws.ThrallMessageSender -import com.gu.mediaservice.lib.management.{ElasticSearchHealthCheck, InnerServiceStatusCheckController, Management} +import com.gu.mediaservice.lib.management.{ElasticSearchHealthCheck, Management} import com.gu.mediaservice.lib.metadata.SoftDeletedMetadataTable import com.gu.mediaservice.lib.play.GridComponents import controllers._ @@ -36,7 +36,6 @@ class MediaApiComponents(context: Context) extends GridComponents(context, new M val usageController = new UsageController(auth, config, elasticSearch, usageQuota, controllerComponents) val elasticSearchHealthCheck = new ElasticSearchHealthCheck(controllerComponents, elasticSearch) val healthcheckController = new Management(controllerComponents, buildInfo) - val InnerServiceStatusCheckController = new InnerServiceStatusCheckController(auth, controllerComponents, config.services, wsClient) override val router = new Routes( httpErrorHandler, @@ -45,7 +44,6 @@ class MediaApiComponents(context: Context) extends GridComponents(context, new M aggController, usageController, elasticSearchHealthCheck, - healthcheckController, - InnerServiceStatusCheckController + healthcheckController ) } diff --git a/media-api/conf/routes b/media-api/conf/routes index d868fd4c40..8d084f6614 100644 --- a/media-api/conf/routes +++ b/media-api/conf/routes @@ -43,7 +43,6 @@ GET /usage/quotas/:id controllers. GET /management/healthcheck com.gu.mediaservice.lib.management.ElasticSearchHealthCheck.healthCheck GET /management/manifest com.gu.mediaservice.lib.management.Management.manifest GET /management/imageCounts com.gu.mediaservice.lib.management.ElasticSearchHealthCheck.imageCounts -GET /management/whoAmI com.gu.mediaservice.lib.management.InnerServiceStatusCheckController.whoAmI(depth: Int) # Shoo robots away GET /robots.txt com.gu.mediaservice.lib.management.Management.disallowRobots diff --git a/metadata-editor/app/MetadataEditorComponents.scala b/metadata-editor/app/MetadataEditorComponents.scala index 1b2c439ebc..ebdb78f3f1 100644 --- a/metadata-editor/app/MetadataEditorComponents.scala +++ b/metadata-editor/app/MetadataEditorComponents.scala @@ -1,4 +1,3 @@ -import com.gu.mediaservice.lib.management.InnerServiceStatusCheckController import com.gu.mediaservice.lib.play.GridComponents import controllers.{EditsApi, EditsController, SyndicationController} import lib._ @@ -23,10 +22,9 @@ class MetadataEditorComponents(context: Context) extends GridComponents(context, val editsController = new EditsController(auth, editsStore, notifications, config, wsClient, authorisation, controllerComponents) val syndicationController = new SyndicationController(auth, editsStore, syndicationStore, notifications, config, controllerComponents) val controller = new EditsApi(auth, config, authorisation, controllerComponents) - val InnerServiceStatusCheckController = new InnerServiceStatusCheckController(auth, controllerComponents, config.services, wsClient) - override val router = new Routes(httpErrorHandler, controller, editsController, syndicationController, management, InnerServiceStatusCheckController) + override val router = new Routes(httpErrorHandler, controller, editsController, syndicationController, management) } diff --git a/metadata-editor/conf/routes b/metadata-editor/conf/routes index 0b12d2f0e0..5aab6e425f 100644 --- a/metadata-editor/conf/routes +++ b/metadata-editor/conf/routes @@ -33,7 +33,6 @@ DELETE /metadata/:id/syndication controllers.SyndicationC # Management GET /management/healthcheck com.gu.mediaservice.lib.management.Management.healthCheck GET /management/manifest com.gu.mediaservice.lib.management.Management.manifest -GET /management/whoAmI com.gu.mediaservice.lib.management.InnerServiceStatusCheckController.whoAmI(depth: Int) # Shoo robots away GET /robots.txt com.gu.mediaservice.lib.management.Management.disallowRobots diff --git a/rest-lib/src/main/scala/com/gu/mediaservice/lib/management/InnerServiceStatusCheckController.scala b/rest-lib/src/main/scala/com/gu/mediaservice/lib/management/InnerServiceStatusCheckController.scala index e313e238ba..e69de29bb2 100644 --- a/rest-lib/src/main/scala/com/gu/mediaservice/lib/management/InnerServiceStatusCheckController.scala +++ b/rest-lib/src/main/scala/com/gu/mediaservice/lib/management/InnerServiceStatusCheckController.scala @@ -1,67 +0,0 @@ -package com.gu.mediaservice.lib.management - -import com.gu.mediaservice.lib.argo.ArgoHelpers -import com.gu.mediaservice.lib.auth.Authentication -import com.gu.mediaservice.lib.auth.Authentication.InnerServicePrincipal -import com.gu.mediaservice.lib.config.Services -import play.api.libs.json.{JsString, JsValue, Json, Writes} -import play.api.libs.ws.{WSClient, WSRequest} -import play.api.mvc.{BaseController, ControllerComponents} - -import scala.concurrent.{ExecutionContext, Future} -import scala.util.Try - -case class WhoAmIResponse (baseUri: String, status: Int, body: JsValue) -object WhoAmIResponse { implicit val writes: Writes[WhoAmIResponse] = Json.writes[WhoAmIResponse] } - -class InnerServiceStatusCheckController( - auth: Authentication, - override val controllerComponents: ControllerComponents, - services: Services, - ws: WSClient -)(implicit ec: ExecutionContext) - extends BaseController with ArgoHelpers { - - private def safeJsonParse(maybeJsonStr: String) = Try(Json.parse(maybeJsonStr)).getOrElse(JsString(maybeJsonStr)) - - private def callAllInternalServices(depth: Int, authenticator: WSRequest => WSRequest) = { - val nextDepth = depth - 1 - val whoAmIFutures = services.allInternalUris.map { baseUri => - authenticator(ws.url(s"$baseUri/management/whoAmI").addQueryStringParameters("depth" -> nextDepth.toString)).get() - .map(resp => WhoAmIResponse(baseUri, resp.status, safeJsonParse(resp.body))) - .recover{ - case throwable: Throwable => WhoAmIResponse(baseUri, SERVICE_UNAVAILABLE, Json.obj( - "errorMessage" -> throwable.getMessage, - "stackTrace" -> throwable.getStackTrace.map(_.toString) - ))} - } - - Future.sequence(whoAmIFutures).map { whoAmIResponses => - val overallStatus = whoAmIResponses.map(_.status).max - new Status(overallStatus)(Json.toJson(whoAmIResponses.map(resp => resp.baseUri -> resp).toMap)) - } - } - - def whoAmI(depth: Int) = auth.async { request => - if (depth < 0 || depth > 2) { Future.successful(BadRequest("'depth' query param must be at least 0 and no more than 2"))} - else request.user match { - case principal: InnerServicePrincipal if depth > 0 => - callAllInternalServices( - depth, - authenticator = auth.getOnBehalfOfPrincipal(principal) - ) - case _ => - Future.successful( - Ok(Json.toJson(request.user.toString)) - ) - } - } - - def statusCheck(depth: Int) = Action.async { - if (depth < 1 || depth > 3) { Future.successful(BadRequest("'depth' query param must be at least 1 and no more than 3"))} - else callAllInternalServices( - depth, - authenticator = auth.innerServiceCall - ) - } -} diff --git a/thrall/app/ThrallComponents.scala b/thrall/app/ThrallComponents.scala index 8ef49988fb..1540d3da6c 100644 --- a/thrall/app/ThrallComponents.scala +++ b/thrall/app/ThrallComponents.scala @@ -3,7 +3,6 @@ import org.apache.pekko.stream.scaladsl.Source import com.gu.kinesis.{KinesisRecord, KinesisSource, ConsumerConfig => KclPekkoStreamConfig} import com.gu.mediaservice.GridClient import com.gu.mediaservice.lib.aws.{S3Ops, ThrallMessageSender} -import com.gu.mediaservice.lib.management.InnerServiceStatusCheckController import com.gu.mediaservice.lib.metadata.SoftDeletedMetadataTable import com.gu.mediaservice.lib.play.GridComponents import com.typesafe.scalalogging.StrictLogging @@ -86,7 +85,6 @@ class ThrallComponents(context: Context) extends GridComponents(context, new Thr val thrallController = new ThrallController(es, store, migrationSourceWithSender.send, messageSender, actorSystem, auth, config.services, controllerComponents, gridClient) val reaperController = new ReaperController(es, store, authorisation, config, actorSystem.scheduler, maybeCustomReapableEligibility, softDeletedMetadataTable, thrallMetrics, auth, config.services, controllerComponents) val healthCheckController = new HealthCheck(es, streamRunning.isCompleted, config, controllerComponents) - val InnerServiceStatusCheckController = new InnerServiceStatusCheckController(auth, controllerComponents, config.services, wsClient) - override lazy val router = new Routes(httpErrorHandler, thrallController, reaperController, healthCheckController, management, InnerServiceStatusCheckController, assets) + override lazy val router = new Routes(httpErrorHandler, thrallController, reaperController, healthCheckController, management, assets) } diff --git a/thrall/conf/routes b/thrall/conf/routes index f2e4274022..46def56903 100644 --- a/thrall/conf/routes +++ b/thrall/conf/routes @@ -39,8 +39,6 @@ POST /resumeReaper controllers.ReaperControll # Management GET /management/healthcheck controllers.HealthCheck.healthCheck GET /management/manifest com.gu.mediaservice.lib.management.Management.manifest -GET /management/innerServiceStatusCheck com.gu.mediaservice.lib.management.InnerServiceStatusCheckController.statusCheck(depth: Int) -GET /management/whoAmI com.gu.mediaservice.lib.management.InnerServiceStatusCheckController.whoAmI(depth: Int) GET /assets/*file controllers.Assets.versioned(path="/public", file: Asset) diff --git a/usage/app/UsageComponents.scala b/usage/app/UsageComponents.scala index 692e10502e..bb7d7f3c43 100644 --- a/usage/app/UsageComponents.scala +++ b/usage/app/UsageComponents.scala @@ -1,5 +1,4 @@ import com.gu.contentapi.client.ScheduledExecutor -import com.gu.mediaservice.lib.management.InnerServiceStatusCheckController import com.gu.mediaservice.lib.play.GridComponents import controllers.UsageApi import lib._ @@ -35,8 +34,7 @@ class UsageComponents(context: Context) extends GridComponents(context, new Usag }) val controller = new UsageApi(auth, authorisation, usageTable, usageGroupOps, notifications, config, usageRecorder.usageApiSubject, liveContentApi, controllerComponents, playBodyParsers) - val InnerServiceStatusCheckController = new InnerServiceStatusCheckController(auth, controllerComponents, config.services, wsClient) - override lazy val router = new Routes(httpErrorHandler, controller, management, InnerServiceStatusCheckController) + override lazy val router = new Routes(httpErrorHandler, controller, management) } diff --git a/usage/conf/routes b/usage/conf/routes index 111eed8374..8e8ceea206 100644 --- a/usage/conf/routes +++ b/usage/conf/routes @@ -14,7 +14,6 @@ GET /usages/digital/content/*contentId/reindex controllers.UsageApi.rei # Management GET /management/healthcheck com.gu.mediaservice.lib.management.Management.healthCheck GET /management/manifest com.gu.mediaservice.lib.management.Management.manifest -GET /management/whoAmI com.gu.mediaservice.lib.management.InnerServiceStatusCheckController.whoAmI(depth: Int) # Shoo robots away GET /robots.txt com.gu.mediaservice.lib.management.Management.disallowRobots From cc487115580b975cf0c1335a404b90a3f128c9d7 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Tue, 25 Jun 2024 13:17:55 +0100 Subject: [PATCH 21/27] Simplify reaper paused control to set by config only. Removes a bucket dependency. --- thrall/app/controllers/ReaperController.scala | 26 ++++--------------- thrall/app/lib/ThrallConfig.scala | 1 + thrall/app/views/reaper.scala.html | 6 ----- thrall/conf/routes | 4 --- 4 files changed, 6 insertions(+), 31 deletions(-) diff --git a/thrall/app/controllers/ReaperController.scala b/thrall/app/controllers/ReaperController.scala index cf2b786c12..7d71eb99b4 100644 --- a/thrall/app/controllers/ReaperController.scala +++ b/thrall/app/controllers/ReaperController.scala @@ -35,9 +35,8 @@ class ReaperController( override val controllerComponents: ControllerComponents, )(implicit val ec: ExecutionContext) extends BaseControllerWithLoginRedirects with GridLogging { - private val CONTROL_FILE_NAME = "PAUSED" - private val INTERVAL = config.reaperInterval //default 15 minutes, based on max of 1000 per reap, this interval will max out at 96,000 images per day + private val isPaused = config.reaperPaused implicit val logMarker: MarkerMap = MarkerMap() @@ -48,14 +47,14 @@ class ReaperController( } } - (config.maybeReaperBucket, config.maybeReaperCountPerRun) match { - case (Some(reaperBucket), Some(countOfImagesToReap)) => + config.maybeReaperCountPerRun match { + case Some(countOfImagesToReap) => scheduler.scheduleAtFixedRate( initialDelay = DateTimeUtils.timeUntilNextInterval(INTERVAL), // so we always start on multiples of the interval past the hour interval = INTERVAL, ){ () => try { - if (store.client.doesObjectExist(reaperBucket, CONTROL_FILE_NAME)) { + if (isPaused) { logger.info("Reaper is paused") es.countTotalSoftReapable(isReapable).map(metrics.softReapable.increment(Nil, _)) es.countTotalHardReapable(isReapable, config.hardReapImagesAge).map(metrics.hardReapable.increment(Nil, _)) @@ -73,7 +72,7 @@ class ReaperController( case NonFatal(e) => logger.error("Reap failed", e) } } - case _ => logger.info("scheduled reaper will not run since 's3.reaper.bucket' and 'reaper.countPerRun' need to be configured in thrall.conf") + case _ => logger.info("scheduled reaper will not run because 'reaper.countPerRun' needs to be configured in thrall.conf") } private def batchDeleteWrapper(count: Int)(func: (Int, String) => Future[JsValue]) = auth.async { request => @@ -173,7 +172,6 @@ class ReaperController( case (None, _) => NotImplemented("'s3.reaper.bucket' not configured in thrall.conf") case (_, None) => NotImplemented("'reaper.countPerRun' not configured in thrall.conf") case (Some(reaperBucket), Some(countOfImagesToReap)) => - val isPaused = store.client.doesObjectExist(reaperBucket, CONTROL_FILE_NAME) 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 ++ @@ -197,18 +195,4 @@ class ReaperController( ).as(JSON) }} - def pauseReaper = auth { config.maybeReaperBucket match { - case None => NotImplemented("Reaper bucket not configured") - case Some(reaperBucket) => - store.client.putObject(reaperBucket, CONTROL_FILE_NAME, "") - Redirect(routes.ReaperController.index) - }} - - def resumeReaper = auth { config.maybeReaperBucket match { - case None => NotImplemented("Reaper bucket not configured") - case Some(reaperBucket) => - store.client.deleteObject(reaperBucket, CONTROL_FILE_NAME) - Redirect(routes.ReaperController.index) - }} - } diff --git a/thrall/app/lib/ThrallConfig.scala b/thrall/app/lib/ThrallConfig.scala index 4385661961..110536effb 100644 --- a/thrall/app/lib/ThrallConfig.scala +++ b/thrall/app/lib/ThrallConfig.scala @@ -71,6 +71,7 @@ class ThrallConfig(resources: GridConfigResources) extends CommonConfigWithElast val projectionParallelism: Int = intDefault("thrall.projection.parallelism", 1) val reaperInterval: FiniteDuration = intDefault("reaper.interval", 15) minutes + val reaperPaused: Boolean = false val hardReapImagesAge: Int = intDefault("reaper.hard.daysInSoftDelete", 14) // soft deleted images age to be hard deleted by Reaper Controller def kinesisConfig: KinesisReceiverConfig = KinesisReceiverConfig(thrallKinesisStream, rewindFrom, this) diff --git a/thrall/app/views/reaper.scala.html b/thrall/app/views/reaper.scala.html index 45967f4387..797d5ae2a9 100644 --- a/thrall/app/views/reaper.scala.html +++ b/thrall/app/views/reaper.scala.html @@ -27,14 +27,8 @@

Reaper

@if(isPaused) {

Reaper is currently paused.

-
- -
} else {

Reaper is currently running (up to @count images every @interval)

-
- -
}

Records from last 48 hours (UTC timestamps)

diff --git a/thrall/conf/routes b/thrall/conf/routes index 46def56903..ff12836a9e 100644 --- a/thrall/conf/routes +++ b/thrall/conf/routes @@ -31,10 +31,6 @@ GET /reaper controllers.ReaperControll GET /reaper/:key controllers.ReaperController.reaperRecord(key: String) DELETE /doBatchSoftReap controllers.ReaperController.doBatchSoftReap(count: Int) DELETE /doBatchHardReap controllers.ReaperController.doBatchHardReap(count: Int) -+nocsrf -POST /pauseReaper controllers.ReaperController.pauseReaper -+nocsrf -POST /resumeReaper controllers.ReaperController.resumeReaper # Management GET /management/healthcheck controllers.HealthCheck.healthCheck From 9482487726eb4ca5347b716f444c6d7391080cab Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Fri, 17 May 2024 14:17:51 +0100 Subject: [PATCH 22/27] Drop more usages composer references; drops composer url config requirement. Remove usages UsageGroups from Guardian Content methods. Remove usages CAPI client and it's config. Delete usages streaming mode Crier steam listening and CAPI specific reindexForContent end point. Container consumers are never going to be Guardian internal. External users would integrate their streams by having a microapp or Lambda ping the instance specific usages API. We can drop this Guardian specific code and config. --- usage/app/UsageComponents.scala | 12 +- usage/app/controllers/UsageApi.scala | 31 ---- usage/app/lib/ContentApis.scala | 78 --------- usage/app/lib/CrierEventProcessor.scala | 193 ----------------------- usage/app/lib/CrierStreamReader.scala | 132 ---------------- usage/app/lib/UsageConfig.scala | 65 -------- usage/app/lib/UsageMetadataBuilder.scala | 34 ---- usage/app/lib/UsageRecorder.scala | 2 +- usage/app/model/ContentWrapper.scala | 22 --- usage/app/model/UsageGroup.scala | 92 +---------- usage/conf/routes | 1 - 11 files changed, 6 insertions(+), 656 deletions(-) delete mode 100644 usage/app/lib/UsageMetadataBuilder.scala delete mode 100644 usage/app/model/ContentWrapper.scala diff --git a/usage/app/UsageComponents.scala b/usage/app/UsageComponents.scala index bb7d7f3c43..a6cbfc08b4 100644 --- a/usage/app/UsageComponents.scala +++ b/usage/app/UsageComponents.scala @@ -12,28 +12,20 @@ class UsageComponents(context: Context) extends GridComponents(context, new Usag final override val buildInfo = utils.buildinfo.BuildInfo - val usageMetadataBuilder = new UsageMetadataBuilder(config) - val mediaWrapper = new MediaWrapperOps(usageMetadataBuilder) - val liveContentApi = new LiveContentApi(config)(ScheduledExecutor()) - val usageGroupOps = new UsageGroupOps(config, mediaWrapper) + val usageGroupOps = new UsageGroupOps(config) val usageTable = new UsageTable(config) val usageMetrics = new UsageMetrics(config, actorSystem, applicationLifecycle) val usageNotifier = new UsageNotifier(config, usageTable) val usageRecorder = new UsageRecorder(usageMetrics, usageTable, usageNotifier, usageNotifier) val notifications = new Notifications(config) - if(!config.apiOnly) { - val crierReader = new CrierStreamReader(config, usageGroupOps, executionContext) - crierReader.start() - } - usageRecorder.start() context.lifecycle.addStopHook(() => { usageRecorder.stop() Future.successful(()) }) - val controller = new UsageApi(auth, authorisation, usageTable, usageGroupOps, notifications, config, usageRecorder.usageApiSubject, liveContentApi, controllerComponents, playBodyParsers) + val controller = new UsageApi(auth, authorisation, usageTable, usageGroupOps, notifications, config, usageRecorder.usageApiSubject, controllerComponents, playBodyParsers) override lazy val router = new Routes(httpErrorHandler, controller, management) diff --git a/usage/app/controllers/UsageApi.scala b/usage/app/controllers/UsageApi.scala index 60ed5c0baf..5bdebe83c8 100644 --- a/usage/app/controllers/UsageApi.scala +++ b/usage/app/controllers/UsageApi.scala @@ -29,7 +29,6 @@ class UsageApi( notifications: Notifications, config: UsageConfig, usageApiSubject: Subject[WithLogMarker[UsageGroup]], - liveContentApi: LiveContentApi, override val controllerComponents: ControllerComponents, playBodyParsers: PlayBodyParsers )( @@ -97,36 +96,6 @@ class UsageApi( } - def reindexForContent(contentId: String) = auth.async { req => - implicit val logMarker: LogMarker = MarkerMap( - "requestType" -> "reindex-for-content", - "requestId" -> RequestLoggingFilter.getRequestId(req), - "contentId" -> contentId, - ) - - val query = liveContentApi.usageQuery(contentId) - - liveContentApi.getResponse(query).map{response => - response.content match { - case Some(content) => - ContentHelpers - .getContentFirstPublished(content) - .map(LiveContentItem(content, _)) - .map(_.copy(isReindex = true)) - .foreach(_.emitAsUsageGroup( - usageApiSubject, - usageGroupOps - )) - Accepted - case _ => - NotFound - } - }.recover { case error: Exception => - logger.error(logMarker, s"UsageApi reindex for content ($contentId) failed!", error) - InternalServerError - } - } - def forMedia(mediaId: String) = auth.async { req => implicit val logMarker: LogMarker = MarkerMap( "requestType" -> "usages-for-media-id", diff --git a/usage/app/lib/ContentApis.scala b/usage/app/lib/ContentApis.scala index 4ce51ce690..e69de29bb2 100644 --- a/usage/app/lib/ContentApis.scala +++ b/usage/app/lib/ContentApis.scala @@ -1,78 +0,0 @@ -package lib - -import com.amazonaws.auth.profile.ProfileCredentialsProvider -import com.amazonaws.auth.{AWSCredentialsProvider, AWSCredentialsProviderChain, STSAssumeRoleSessionCredentialsProvider} -import com.amazonaws.services.securitytoken.{AWSSecurityTokenService, AWSSecurityTokenServiceClientBuilder} -import com.gu.contentapi.client._ -import com.gu.contentapi.client.model.{HttpResponse, ItemQuery} - -import java.net.URI -import scala.concurrent.duration.DurationInt -import scala.concurrent.{ExecutionContext, Future} - -abstract class UsageContentApiClient(config: UsageConfig)(implicit val executor: ScheduledExecutor) - extends GuardianContentClient(apiKey = config.capiApiKey) { - - def usageQuery(contentId: String): ItemQuery = { - ItemQuery(contentId) - .showFields("firstPublicationDate,isLive,internalComposerCode") - .showElements("image,cartoon") - .showAtoms("media") - } -} - -class LiveContentApi(config: UsageConfig)(implicit val ex: ScheduledExecutor) - extends UsageContentApiClient(config) with RetryableContentApiClient { - - override val targetUrl: String = config.capiLiveUrl - override val backoffStrategy: BackoffStrategy = BackoffStrategy.doublingStrategy(2.seconds, config.capiMaxRetries) -} - -class PreviewContentApi(protected val config: UsageConfig)(implicit val ex: ScheduledExecutor) - // ensure IAMAuthContentApiClient is the first trait in this list! - extends UsageContentApiClient(config) with IAMAuthContentApiClient with RetryableContentApiClient { - - override val targetUrl: String = config.capiPreviewUrl - override val backoffStrategy: BackoffStrategy = BackoffStrategy.doublingStrategy(2.seconds, config.capiMaxRetries) -} - -// order of mixing is important. Some client traits (notably RetryableContentApiClient!) -// also override get, adding header(s) (and could potentially edit the uri too) before calling super.get(). Those -// traits must be executed BEFORE this trait, so that the get override in this trait -// receives the headers that will actually be sent over the wire. -// so any class mixing this in should have it first in the list of traits, eg. -// class MyCapiClient extends GuardianContentApiClient(apiKey) -// with IAMAuthContentApiClient with RetryableContentApiClient with MyOtherClientTraits -// ie. the super calls will travel "from right to left" along the trait list, and this trait can sign the accumulated headers -trait IAMAuthContentApiClient extends ContentApiClient { - protected val config: UsageConfig - - lazy val sts: AWSSecurityTokenService = AWSSecurityTokenServiceClientBuilder.standard() - .withRegion(config.awsRegionName) - .build() - - lazy val capiCredentials: AWSCredentialsProvider = - config.capiPreviewRole.map( - new STSAssumeRoleSessionCredentialsProvider.Builder(_, "capi") - .withStsClient(sts) - .build() - ).getOrElse(new ProfileCredentialsProvider("capi")) // will be used if stream is ever run locally (unusual) - - abstract override def get( - url: String, - headers: Map[String, String] - )(implicit context: ExecutionContext): Future[HttpResponse] = { - - val uri = new URI(url) - val encodedQuery = IAMEncoder.encodeParams(uri.getQuery) - - // no mutation of uris, and no easy way to create from a given one - val encodedUri = new URI(uri.getScheme, uri.getAuthority, uri.getPath, encodedQuery, uri.getFragment) - - val signer = new IAMSigner(capiCredentials, config.awsRegionName) - - val withIamHeaders = signer.addIAMHeaders(headers, encodedUri) - - super.get(encodedUri.toString, withIamHeaders) - } -} diff --git a/usage/app/lib/CrierEventProcessor.scala b/usage/app/lib/CrierEventProcessor.scala index abf46e069e..e69de29bb2 100644 --- a/usage/app/lib/CrierEventProcessor.scala +++ b/usage/app/lib/CrierEventProcessor.scala @@ -1,193 +0,0 @@ -package lib - -import com.gu.contentapi.client.ScheduledExecutor -import com.gu.contentapi.client.model.ContentApiError -import com.gu.contentapi.client.model.v1.Content -import com.gu.crier.model.event.v1.{Event, EventPayload, EventType} -import com.gu.mediaservice.lib.logging.{GridLogging, LogMarker, MarkerMap} -import com.gu.mediaservice.model.usage.{PendingUsageStatus, PublishedUsageStatus} -import com.gu.thrift.serializer.ThriftDeserializer -import com.twitter.scrooge.ThriftStructCodec -import model.{UsageGroup, UsageGroupOps} -import org.joda.time.DateTime -import rx.lang.scala.Subject -import rx.lang.scala.subjects.PublishSubject -import software.amazon.kinesis.exceptions.ShutdownException -import software.amazon.kinesis.leases.exceptions.InvalidStateException -import software.amazon.kinesis.lifecycle.events._ -import software.amazon.kinesis.processor.ShardRecordProcessor - -import java.util.UUID -import scala.concurrent.ExecutionContext.Implicits.global -import scala.jdk.CollectionConverters._ -import scala.util.Try - -trait ContentContainer extends GridLogging { - val content: Content - val lastModified: DateTime - val isReindex: Boolean - - private lazy val isEntirePieceTakenDown = - content.fields.exists(fields => fields.firstPublicationDate.isDefined && fields.isLive.contains(false)) - - def emitAsUsageGroup( - publishSubject: Subject[WithLogMarker[UsageGroup]], usageGroupOps: UsageGroupOps - )(implicit logMarker: LogMarker): Unit = { - usageGroupOps.build( - content, - status = this match { - case PreviewContentItem(_,_,_) => PendingUsageStatus - case LiveContentItem(_,_,_) => PublishedUsageStatus - }, - lastModified, - isReindex - ) match { - case None => logger.debug(logMarker, s"No fields in content of crier update for payload with content ID ${content.id}") - case Some(usageGroup) => - val groupingLogMarker = logMarker ++ Map("usageGroup" -> usageGroup.grouping) - - publishSubject.onNext(WithLogMarker(groupingLogMarker, usageGroup)) - - if (this.isInstanceOf[PreviewContentItem] && isEntirePieceTakenDown) { - logger.info(groupingLogMarker, s"${usageGroup.grouping} is taken down so producing empty UsageGroup to ensure any 'published' DB records are marked as removed") - publishSubject.onNext(WithLogMarker(groupingLogMarker, usageGroup.copy( - usages = Set.empty, - maybeStatus = Some(PublishedUsageStatus) - ))) - } - } - } -} - -object CrierUsageStream { - val observable: Subject[WithLogMarker[UsageGroup]] = PublishSubject[WithLogMarker[UsageGroup]]() -} - -case class LiveContentItem(content: Content, lastModified: DateTime, isReindex: Boolean = false) extends ContentContainer -case class PreviewContentItem(content: Content, lastModified: DateTime, isReindex: Boolean = false) extends ContentContainer - -abstract class CrierEventProcessor(config: UsageConfig, usageGroupOps: UsageGroupOps) extends ShardRecordProcessor with GridLogging { - - implicit val codec: ThriftStructCodec[Event] = Event - - val contentApiClient: UsageContentApiClient - - override def initialize(initializationInput: InitializationInput): Unit = { - logger.debug(s"Initialized an event processor for shard ${initializationInput.shardId}") - } - - override def processRecords(processRecordsInput: ProcessRecordsInput): Unit = { - val records = processRecordsInput.records - records.asScala.foreach { record => - val deserialization: Try[Event] = ThriftDeserializer.deserialize(record.data) - deserialization.foreach(processEvent) - deserialization.failed.foreach { e: Throwable => - logger.error("Failed to deserialize crier event", e) - } - } - - val lastRecord = records.asScala.last - - processRecordsInput.checkpointer.checkpoint(lastRecord.sequenceNumber(), lastRecord.subSequenceNumber()) - } - - override def leaseLost(leaseLostInput: LeaseLostInput): Unit = { - // nothing to do? - logger.debug("Lost lease, so stopping processing Crier") - } - - override def shardEnded(shardEndedInput: ShardEndedInput): Unit = { - try { - shardEndedInput.checkpointer.checkpoint() - logger.debug("Shard ended, so stopping processing Crier") - } catch { - case _: ShutdownException | _: InvalidStateException => - () - } - } - - override def shutdownRequested(shutdownRequestedInput: ShutdownRequestedInput): Unit = { - try { - shutdownRequestedInput.checkpointer.checkpoint() - logger.debug("Shutdown requested, so stopping processing Crier") - } catch { - case _: ShutdownException | _: InvalidStateException => - () - } - } - - def getContentItem(content: Content, time: DateTime): ContentContainer - - - private def processEvent(event: Event): Unit = { - implicit val logMarker: LogMarker = MarkerMap( - "payloadId" -> event.payloadId, - "requestId" -> UUID.randomUUID().toString - ) - - Try { - val dateTime: DateTime = new DateTime(event.dateTime) - - event.eventType match { - case EventType.Update => - - event.payload match { - case Some(content: EventPayload.Content) => - getContentItem(content.content, dateTime) - .emitAsUsageGroup(CrierUsageStream.observable, usageGroupOps) - case _ => - logger.warn(logMarker, s"Received crier update for ${event.payloadId} without payload") - } - case EventType.Delete => - //TODO: how do we deal with a piece of content that has been deleted? - case EventType.RetrievableUpdate => - - event.payload match { - case Some(retrievableContent: EventPayload.RetrievableContent) => - val capiUrl = retrievableContent.retrievableContent.capiUrl - - val query = contentApiClient.usageQuery(retrievableContent.retrievableContent.id) - - logger.info(logMarker, s"retrieving content event at $capiUrl parsed to id ${query.toString}") - - contentApiClient.getResponse(query).map(response => { - response.content match { - case Some(content) => - getContentItem(content, dateTime) - .emitAsUsageGroup(CrierUsageStream.observable, usageGroupOps) - case _ => - logger.debug( - logMarker, - s"Received retrievable update for ${retrievableContent.retrievableContent.id} without content" - ) - } - }).recover { - case e: ContentApiError => - logger.error(logMarker, s"CAPI error when fetching content update for ${event.payloadId}: ${e.httpStatus} ${e.httpMessage} ${e.errorResponse}", e) - case e => - logger.error(logMarker, s"Failed to fetch or process content update for ${event.payloadId}", e) - } - case _ => logger.warn(logMarker, s"Received crier update for ${event.payloadId} without payload") - } - - case _ => logger.warn(logMarker, s"Unsupported event type $EventType") - } - }.recover { - case e => logger.error(logMarker, s"Failed to process event ${event.payloadId}", e) - } - } -} - -private class CrierLiveEventProcessor(config: UsageConfig, usageGroupOps: UsageGroupOps) extends CrierEventProcessor(config, usageGroupOps) { - - def getContentItem(content: Content, date: DateTime): ContentContainer = LiveContentItem(content, date) - - override val contentApiClient: LiveContentApi = new LiveContentApi(config)(ScheduledExecutor()) -} - -private class CrierPreviewEventProcessor(config: UsageConfig, usageGroupOps: UsageGroupOps) extends CrierEventProcessor(config, usageGroupOps) { - - def getContentItem(content: Content, date: DateTime): ContentContainer = PreviewContentItem(content, date) - - override val contentApiClient: PreviewContentApi = new PreviewContentApi(config)(ScheduledExecutor()) -} diff --git a/usage/app/lib/CrierStreamReader.scala b/usage/app/lib/CrierStreamReader.scala index 2c4b42c486..e69de29bb2 100644 --- a/usage/app/lib/CrierStreamReader.scala +++ b/usage/app/lib/CrierStreamReader.scala @@ -1,132 +0,0 @@ -package lib - -import com.gu.mediaservice.lib.logging.GridLogging -import model.UsageGroupOps -import software.amazon.awssdk.auth.credentials.{AwsCredentialsProviderChain, DefaultCredentialsProvider, ProfileCredentialsProvider} -import software.amazon.awssdk.regions.Region -import software.amazon.awssdk.services.cloudwatch.CloudWatchAsyncClient -import software.amazon.awssdk.services.dynamodb.DynamoDbAsyncClient -import software.amazon.awssdk.services.kinesis.KinesisAsyncClient -import software.amazon.awssdk.services.sts.StsClient -import software.amazon.awssdk.services.sts.auth.StsAssumeRoleCredentialsProvider -import software.amazon.awssdk.services.sts.model.AssumeRoleRequest -import software.amazon.kinesis.common.{ConfigsBuilder, InitialPositionInStream, InitialPositionInStreamExtended, KinesisClientUtil} -import software.amazon.kinesis.coordinator.CoordinatorConfig.ClientVersionConfig -import software.amazon.kinesis.coordinator.Scheduler -import software.amazon.kinesis.processor.{ShardRecordProcessor, ShardRecordProcessorFactory} -import software.amazon.kinesis.retrieval.polling.PollingConfig - -import java.net.InetAddress -import java.util.UUID -import scala.annotation.nowarn -import scala.concurrent.ExecutionContext - -// it's annoyingly hard to get the streamName out of the configsBuilder once built, so pass them around together -private case class ConfigsBuilderWithStreamName(configsBuilder: ConfigsBuilder, streamName: String) - -class CrierStreamReader( - config: UsageConfig, - usageGroupOps: UsageGroupOps, - executionContext: ExecutionContext -) extends GridLogging { - - private val region = Region.of(config.awsRegionName) - - private lazy val workerId: String = InetAddress.getLocalHost.getCanonicalHostName + ":" + UUID.randomUUID() - - private lazy val awsCredentialsProvider = DefaultCredentialsProvider.builder().profileName("media-service").build() - private lazy val stsClient = StsClient.builder().region(region).credentialsProvider(awsCredentialsProvider).build() - - private lazy val sessionId: String = "session-" + Math.random() - private val initialPosition = InitialPositionInStreamExtended.newInitialPosition(InitialPositionInStream.TRIM_HORIZON) - - private def kinesisCredentialsProvider(arn: String): AwsCredentialsProviderChain = { - val assumeRoleRequest = AssumeRoleRequest.builder().roleArn(arn).roleSessionName(sessionId).build() - - AwsCredentialsProviderChain.of( - ProfileCredentialsProvider.create("capi"), - StsAssumeRoleCredentialsProvider.builder().refreshRequest(assumeRoleRequest).stsClient(stsClient).build() - ) - } - - private def kinesisClientLibConfig(processorFactory: ShardRecordProcessorFactory) - (kinesisReaderConfig: KinesisReaderConfig): ConfigsBuilderWithStreamName = { - - val kinesisClient = KinesisClientUtil.createKinesisAsyncClient(KinesisAsyncClient.builder() - .region(region) - .credentialsProvider(kinesisCredentialsProvider(kinesisReaderConfig.arn))) - val dynamoClient = DynamoDbAsyncClient.builder() - .region(region) - .credentialsProvider(awsCredentialsProvider) - .build() - val cloudwatchClient = CloudWatchAsyncClient.builder() - .region(region) - .credentialsProvider(awsCredentialsProvider) - .build() - - ConfigsBuilderWithStreamName( - new ConfigsBuilder( - kinesisReaderConfig.streamName, - kinesisReaderConfig.appName, - kinesisClient, - dynamoClient, - cloudwatchClient, - workerId, - processorFactory - ), - kinesisReaderConfig.streamName - ) - } - - @nowarn("cat=deprecation") // initialPositionInStreamExtended is deprecated, but the upgrade path is unclear - private def kinesisClientLibScheduler(configsBuilderAndStreamName: ConfigsBuilderWithStreamName): Scheduler = { - val ConfigsBuilderWithStreamName(configsBuilder, streamName) = configsBuilderAndStreamName - new Scheduler( - configsBuilder.checkpointConfig(), - configsBuilder.coordinatorConfig() - .clientVersionConfig(ClientVersionConfig.CLIENT_VERSION_CONFIG_COMPATIBLE_WITH_2X), - configsBuilder.leaseManagementConfig(), - configsBuilder.lifecycleConfig(), - configsBuilder.metricsConfig(), - configsBuilder.processorConfig(), - configsBuilder.retrievalConfig() - .initialPositionInStreamExtended(initialPosition) - .retrievalSpecificConfig(new PollingConfig(streamName, configsBuilder.kinesisClient())), - ) - } - - private val LiveEventProcessorFactory = new ShardRecordProcessorFactory { - override def shardRecordProcessor(): ShardRecordProcessor = - new CrierLiveEventProcessor(config, usageGroupOps) - } - - private val PreviewEventProcessorFactory = new ShardRecordProcessorFactory { - override def shardRecordProcessor(): ShardRecordProcessor = - new CrierPreviewEventProcessor(config, usageGroupOps) - } - - private lazy val liveConfig = config.liveKinesisReaderConfig - .map(kinesisClientLibConfig(LiveEventProcessorFactory)) - private lazy val previewConfig = config.previewKinesisReaderConfig - .map(kinesisClientLibConfig(PreviewEventProcessorFactory)) - - private lazy val liveScheduler = liveConfig.map(kinesisClientLibScheduler) - private lazy val previewScheduler = previewConfig.map(kinesisClientLibScheduler) - - def start(): Unit = { - logger.info("Trying to start Crier Stream Readers") - - liveScheduler - .map(executionContext.execute) - .fold( - e => logger.error("No 'Crier Live Stream reader' thread to start", e), - _ => logger.info("Starting Crier Live Stream reader") - ) - previewScheduler - .map(executionContext.execute) - .fold( - e => logger.error("No 'Crier Preview Stream reader' thread to start", e), - _ => logger.info("Starting Crier Preview Stream reader") - ) - } -} diff --git a/usage/app/lib/UsageConfig.scala b/usage/app/lib/UsageConfig.scala index 19f07e6e7e..cfbfbdcad9 100644 --- a/usage/app/lib/UsageConfig.scala +++ b/usage/app/lib/UsageConfig.scala @@ -1,84 +1,19 @@ package lib -import com.amazonaws.services.identitymanagement._ import com.gu.mediaservice.lib.config.{CommonConfig, GridConfigResources} import com.gu.mediaservice.lib.logging.GridLogging import com.gu.mediaservice.lib.net.URI.ensureSecure -import scala.util.Try - - -case class KinesisReaderConfig(streamName: String, arn: String, appName: String) - class UsageConfig(resources: GridConfigResources) extends CommonConfig(resources) with GridLogging { val usageUri: String = services.usageBaseUri val apiUri: String = services.apiBaseUri - val defaultMaxRetries = 4 val defaultMaxPrintRequestSizeInKb = 500 val defaultDateLimit = "2016-01-01T00:00:00+00:00" val maxPrintRequestLengthInKb: Int = intDefault("api.setPrint.maxLength", defaultMaxPrintRequestSizeInKb) - val capiLiveUrl = string("capi.live.url") - val capiPreviewUrl = string("capi.preview.url") - val capiPreviewRole = stringOpt("capi.preview.role") - val capiApiKey = string("capi.apiKey") - val capiMaxRetries: Int = intDefault("capi.maxRetries", defaultMaxRetries) - val usageDateLimit: String = stringDefault("usage.dateLimit", defaultDateLimit) - private val composerBaseUrlProperty: String = string("composer.baseUrl") - private val composerBaseUrl = ensureSecure(composerBaseUrlProperty) - - val composerContentBaseUrl: String = s"$composerBaseUrl/content" - val usageRecordTable = string("dynamo.tablename.usageRecordTable") - - val awsRegionName = string("aws.region") - - private val iamClient: AmazonIdentityManagement = withAWSCredentials(AmazonIdentityManagementClientBuilder.standard()).build() - - val postfix: String = if (isDev) { - try { - iamClient.getUser.getUser.getUserName - } catch { - case e:com.amazonaws.SdkClientException => - logger.warn("Unable to determine current IAM user, probably because you're using temp credentials. Usage may not be able to determine the live/preview app names", e) - "tempcredentials" - } - } else { - stage - } - - val liveAppName = s"media-service-livex-$postfix" - val previewAppName = s"media-service-previewx-$postfix" - - val crierLiveKinesisStream = Try { string("crier.live.name") } - val crierPreviewKinesisStream = Try { string("crier.preview.name") } - - val crierLiveArn = Try { string("crier.live.arn") } - val crierPreviewArn = Try { string("crier.preview.arn") } - - val liveKinesisReaderConfig: Try[KinesisReaderConfig] = for { - liveStream <- crierLiveKinesisStream - liveArn <- crierLiveArn - } yield KinesisReaderConfig(liveStream, liveArn, liveAppName) - - val previewKinesisReaderConfig: Try[KinesisReaderConfig] = for { - previewStream <- crierPreviewKinesisStream - previewArn <- crierPreviewArn - } yield KinesisReaderConfig(previewStream, previewArn, previewAppName) - - val apiOnly: Boolean = stringOpt("app.name") match { - case Some("usage-stream") => - logger.info(s"Starting as Stream Reader Usage.") - false - case Some("usage") => - logger.info(s"Starting as API only Usage.") - true - case name => - logger.error(s"App name is invalid: $name") - sys.exit(1) - } } diff --git a/usage/app/lib/UsageMetadataBuilder.scala b/usage/app/lib/UsageMetadataBuilder.scala deleted file mode 100644 index 4f342f7837..0000000000 --- a/usage/app/lib/UsageMetadataBuilder.scala +++ /dev/null @@ -1,34 +0,0 @@ -package lib - -import java.net.URI - -import com.gu.contentapi.client.model.v1.Content -import com.gu.mediaservice.model.usage._ - -import scala.util.Try - -class UsageMetadataBuilder(config: UsageConfig) { - - def composerUrl(content: Content): Option[URI] = content.fields - .flatMap(_.internalComposerCode) - .flatMap(composerId => { - Try(URI.create(s"${config.composerContentBaseUrl}/$composerId")).toOption - }) - - def buildDownload(metadataMap: Map[String, Any]): Option[DownloadUsageMetadata] = { - Try { - DownloadUsageMetadata( - metadataMap("downloadedBy").asInstanceOf[String] - ) - }.toOption - } - - def build(content: Content): DigitalUsageMetadata = { - DigitalUsageMetadata( - URI.create(content.webUrl), - content.webTitle, - content.sectionId.getOrElse("none"), - composerUrl(content) - ) - } -} diff --git a/usage/app/lib/UsageRecorder.scala b/usage/app/lib/UsageRecorder.scala index 5136fe10a9..1a9ed69954 100644 --- a/usage/app/lib/UsageRecorder.scala +++ b/usage/app/lib/UsageRecorder.scala @@ -19,7 +19,7 @@ class UsageRecorder( ) extends GridLogging { val usageApiSubject: Subject[WithLogMarker[UsageGroup]] = PublishSubject[WithLogMarker[UsageGroup]]() - val combinedObservable: Observable[WithLogMarker[UsageGroup]] = CrierUsageStream.observable.merge(usageApiSubject) + val combinedObservable: Observable[WithLogMarker[UsageGroup]] = usageApiSubject val subscriber: Subscriber[LogMarker] = Subscriber((markers: LogMarker) => logger.debug(markers, s"Sent Usage Notification")) var maybeSubscription: Option[Subscription] = None diff --git a/usage/app/model/ContentWrapper.scala b/usage/app/model/ContentWrapper.scala deleted file mode 100644 index b5a074971f..0000000000 --- a/usage/app/model/ContentWrapper.scala +++ /dev/null @@ -1,22 +0,0 @@ -package model - -import com.gu.contentapi.client.model.v1.Content -import com.gu.mediaservice.model.usage.UsageStatus - -import org.joda.time.DateTime - - -case class ContentWrapper( - id: String, - status: UsageStatus, - lastModified: DateTime, - content: Content -) -object ContentWrapper { - def build(content: Content, status: UsageStatus, lastModified: DateTime): Option[ContentWrapper] = { - extractId(content).map(ContentWrapper(_, status, lastModified, content)) - } - - def extractId(content: Content): Option[String] = - content.fields.flatMap(_.internalComposerCode).map(composerId => s"composer/$composerId") -} diff --git a/usage/app/model/UsageGroup.scala b/usage/app/model/UsageGroup.scala index b8d64b6c6e..ce1363f92b 100644 --- a/usage/app/model/UsageGroup.scala +++ b/usage/app/model/UsageGroup.scala @@ -5,11 +5,9 @@ import com.gu.contentapi.client.model.v1.{Content, Element, ElementType} import com.gu.contentatom.thrift.{Atom, AtomData} import com.gu.mediaservice.lib.logging.{GridLogging, LogMarker} import com.gu.mediaservice.model.usage.{DigitalUsageMetadata, MediaUsage, PublishedUsageStatus, UsageStatus} -import lib.{ContentHelpers, MD5, MediaUsageBuilder, UsageConfig, UsageMetadataBuilder} +import lib.{ContentHelpers, MD5, MediaUsageBuilder, UsageConfig} import org.joda.time.DateTime -import scala.collection.compat._ - case class UsageGroup( usages: Set[MediaUsage], grouping: String, @@ -17,10 +15,9 @@ case class UsageGroup( isReindex: Boolean = false, maybeStatus: Option[UsageStatus] = None ) -class UsageGroupOps(config: UsageConfig, mediaWrapperOps: MediaWrapperOps) +class UsageGroupOps(config: UsageConfig) extends GridLogging { - def buildId(contentWrapper: ContentWrapper) = contentWrapper.id def buildId(printUsage: PrintUsageRecord) = s"print/${MD5.hash(List( Some(printUsage.mediaId), Some(printUsage.printUsageMetadata.pageNumber), @@ -51,13 +48,6 @@ class UsageGroupOps(config: UsageConfig, mediaWrapperOps: MediaWrapperOps) ).mkString("_")) }" - def build(content: Content, status: UsageStatus, lastModified: DateTime, isReindex: Boolean)(implicit logMarker: LogMarker) = - ContentWrapper.build(content, status, lastModified).map(contentWrapper => { - val usages = createUsages(contentWrapper, isReindex) - logger.info(logMarker, s"Built UsageGroup: ${contentWrapper.id}") - UsageGroup(usages.toSet, contentWrapper.id, lastModified, isReindex, maybeStatus = Some(status)) - }) - def build(printUsageRecords: List[PrintUsageRecord]) = printUsageRecords.map(printUsageRecord => { val usageId = UsageIdBuilder.build(printUsageRecord) @@ -96,41 +86,6 @@ class UsageGroupOps(config: UsageConfig, mediaWrapperOps: MediaWrapperOps) ) } - def createUsages(contentWrapper: ContentWrapper, isReindex: Boolean)(implicit logMarker: LogMarker) = { - // Generate unique UUID to track extract job - val uuid = java.util.UUID.randomUUID.toString - implicit val extractJobLogMarkers: LogMarker = logMarker ++ Map("extract-job-id" -> uuid) - - val content = contentWrapper.content - val usageStatus = contentWrapper.status - - logger.info(extractJobLogMarkers, s"Extracting images from ${content.id}") - - val mediaAtomsUsages = extractMediaAtoms(content, usageStatus, isReindex)(extractJobLogMarkers).flatMap { atom => - getImageId(atom) match { - case Some(id) => - val mediaWrapper = mediaWrapperOps.build(mediaId = id, contentWrapper = contentWrapper, usageGroupId = buildId(contentWrapper)) - val usage = MediaUsageBuilder.build(mediaWrapper) - Seq(createUsagesLogging(usage)(logMarker)) - case None => Seq.empty - } - } - val imageElementUsages = extractImageElements(content, usageStatus, isReindex)(extractJobLogMarkers).map { element => - val mediaWrapper = mediaWrapperOps.build(mediaId = element.id, contentWrapper = contentWrapper, usageGroupId = buildId(contentWrapper)) - val usage = MediaUsageBuilder.build(mediaWrapper) - createUsagesLogging(usage)(logMarker) - } - val cartoonElementUsages = extractCartoonUniqueMediaIds(content).map { mediaId => - val mediaWrapper = mediaWrapperOps.build(mediaId, contentWrapper = contentWrapper, usageGroupId = buildId(contentWrapper)) - val usage = MediaUsageBuilder.build(mediaWrapper) - createUsagesLogging(usage)(logMarker) - } - - // TODO capture images from interactive embeds - - mediaAtomsUsages ++ imageElementUsages ++ cartoonElementUsages - } - private def createUsagesLogging(usage: MediaUsage)(implicit logMarker: LogMarker) = { logger.info(logMarker, s"Built MediaUsage for ${usage.mediaId}") @@ -153,40 +108,6 @@ class UsageGroupOps(config: UsageConfig, mediaWrapperOps: MediaWrapperOps) } } - private def extractMediaAtoms(content: Content, usageStatus: UsageStatus, isReindex: Boolean)(implicit logMarker: LogMarker) = { - val isNew = isNewContent(content, usageStatus) - val shouldRecordUsages = isNew || isReindex - - if (shouldRecordUsages) { - logger.info(logMarker, s"Passed shouldRecordUsages for media atom") - val groupedMediaAtoms = groupMediaAtoms(content) - - if (groupedMediaAtoms.isEmpty) { - logger.info(logMarker, s"No Matching media atoms found") - } else { - logger.info(logMarker, s"${groupedMediaAtoms.length} media atoms found") - groupedMediaAtoms.foreach(atom => logger.info(logMarker, s"Matching media atom ${atom.id} found")) - } - - groupedMediaAtoms - } else { - logger.info(logMarker, s"Failed shouldRecordUsages for media atoms: isNew-$isNew isReindex-$isReindex") - Seq.empty - } - } - - private def groupMediaAtoms(content: Content) = { - val mediaAtoms = content.atoms match { - case Some(atoms) => - atoms.media match { - case Some(mediaAtoms) => filterOutAtomsWithNoImage(mediaAtoms.toSeq) - case _ => Seq.empty - } - case _ => Seq.empty - } - mediaAtoms - } - private def filterOutAtomsWithNoImage(atoms: Seq[Atom]): Seq[Atom] = { for { atom <- atoms @@ -207,7 +128,7 @@ class UsageGroupOps(config: UsageConfig, mediaWrapperOps: MediaWrapperOps) } } - private def extractCartoonUniqueMediaIds(content: Content): Set[String] = + private def extractCartoonUniqueMediaIds(content: Content): Set[String] = (for { elements <- content.elements.toSeq cartoonElement <- elements.filter(_.`type` == ElementType.Cartoon) @@ -260,10 +181,3 @@ case class MediaWrapper( contentStatus: UsageStatus, usageMetadata: DigitalUsageMetadata, lastModified: DateTime) - -class MediaWrapperOps(usageMetadataBuilder: UsageMetadataBuilder) { - def build(mediaId: String, contentWrapper: ContentWrapper, usageGroupId: String): MediaWrapper = { - val usageMetadata = usageMetadataBuilder.build(contentWrapper.content) - MediaWrapper(mediaId, usageGroupId, contentWrapper.status, usageMetadata, contentWrapper.lastModified) - } -} diff --git a/usage/conf/routes b/usage/conf/routes index 8e8ceea206..1688755800 100644 --- a/usage/conf/routes +++ b/usage/conf/routes @@ -9,7 +9,6 @@ POST /usages/syndication controllers.UsageApi.set POST /usages/front controllers.UsageApi.setFrontUsages() POST /usages/download controllers.UsageApi.setDownloadUsages() PUT /usages/status/update/:mediaId/*usageId controllers.UsageApi.updateUsageStatus(mediaId: String, usageId: String) -GET /usages/digital/content/*contentId/reindex controllers.UsageApi.reindexForContent(contentId: String) # Management GET /management/healthcheck com.gu.mediaservice.lib.management.Management.healthCheck From acad460994f82cf37ba58a088b75b008ae1f99bb Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Wed, 26 Jun 2024 12:53:20 +0100 Subject: [PATCH 23/27] Use imgproxy rather than nginx imgops for optimised image previews. Faster and lighter. Better colour profile support. Generate an imgproxy style preview URL. Move service name from imgops to imgproxy. Disable EXIF autorotation and explicitly correct rotation. imgproxy does not accept negative rotations. Be explicit about stripping colour profile to force sRGB. --- .../gu/mediaservice/lib/config/Services.scala | 2 +- media-api/app/lib/ImageResponse.scala | 31 +++++++++++-------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala index dc031c0641..e46ee68c54 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala @@ -49,7 +49,7 @@ protected class SingleHostServices(val rootUrl: String) extends Services { val metadataBaseUri: String = subpathedServiceBaseUri("metadata-editor") - val imgopsBaseUri: String = subpathedServiceBaseUri("imgops") + val imgopsBaseUri: String = subpathedServiceBaseUri("imgproxy") val usageBaseUri: String =subpathedServiceBaseUri("usage") diff --git a/media-api/app/lib/ImageResponse.scala b/media-api/app/lib/ImageResponse.scala index 1a76a3b6e6..11653d03b9 100644 --- a/media-api/app/lib/ImageResponse.scala +++ b/media-api/app/lib/ImageResponse.scala @@ -10,6 +10,7 @@ import com.gu.mediaservice.model.usage._ 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._ @@ -144,9 +145,9 @@ class ImageResponse(config: MediaApiConfig, s3Client: S3Client, usageQuota: Usag import BoolImplicitMagic.BoolToOption val cropLinkMaybe = valid.toOption(Link("crops", s"${config.cropperUri}/crops/$id")) val editLinkMaybe = withWritePermission.toOption(Link("edits", s"${config.metadataUri}/metadata/$id")) - val optimisedPngLinkMaybe = securePngUrl map { case secureUrl => Link("optimisedPng", makeImgopsUri(new URI(secureUrl), orientationMetadata)) } + val optimisedPngLinkMaybe = securePngUrl map { case secureUrl => Link("optimisedPng", makeImgProxyUri(new URI(secureUrl), orientationMetadata)) } - val optimisedLink = Link("optimised", makeImgopsUri(new URI(secureUrl), orientationMetadata)) + val optimisedLink = Link("optimised", makeImgProxyUri(new URI(secureUrl), orientationMetadata)) val imageLink = Link("ui:image", s"${config.kahunaUri}/images/$id") val usageLink = Link("usages", s"${config.usageUri}/usages/media/$id") val leasesLink = Link("leases", s"${config.leasesUri}/leases/media/$id") @@ -253,18 +254,22 @@ class ImageResponse(config: MediaApiConfig, s3Client: S3Client, usageQuota: Usag "aliases" -> JsObject(aliases) )) - def makeImgopsUri(uri: URI, orientationMetadata: Option[OrientationMetadata]): String = { - val resizing = config.imgopsUri + List(uri.getPath, uri.getRawQuery).mkString("?") + "{&w,h,q}" - // imgops rotates counter-clockwise - val orientationCorrectionRotation = -orientationMetadata.map(_.orientationCorrection()).getOrElse(0) - // and ignores negative values - val normalised = if (orientationCorrectionRotation < 0) { - orientationCorrectionRotation + 360 - } else { - orientationCorrectionRotation + private def makeImgProxyUri(uri: URI, orientationMetadata: Option[OrientationMetadata]): String = { + def normaliseRotation(rotation: Int) = { + // imgproxy does not accept negative rotations + if (rotation < 0) { + rotation + 360 + } else { + rotation + } } - val orientationCorrection = s"&r=" + URLEncoder.encode(normalised.toString, "UTF-8") - resizing + orientationCorrection + val base64EncodedSourceURL = new String(Base64.encodeBase64URLSafe(uri.toURL.toExternalForm.getBytes), "UTF-8") + val resizing = Seq(config.imgopsUri, "no-signature", + "auto_rotate:false", "strip_metadata:true", "strip_color_profile:true", + "resize:fit:{w}:{h}", "quality:{q}") + val orientationCorrection = orientationMetadata.map(o => Seq("rotate:" + normaliseRotation(o.orientationCorrection()))).getOrElse(Seq.empty) + val pathComponents = resizing ++ orientationCorrection :+ base64EncodedSourceURL + pathComponents.mkString("/") } private def updateCustomSpecialInstructions(source: JsValue): Reads[JsObject] = { From ce75077566f98d4f9d400698bb6daf14305c7317 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Tue, 28 May 2024 16:57:40 +0100 Subject: [PATCH 24/27] Play base project heap size set to 50%; more heap for given container size than default ergonomics. 60% was running close to the OOM kill limit on Java 11 and Java 22 has nudged it into OOM kill. --- build.sbt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/build.sbt b/build.sbt index 71daba3055..60d5dc6b8d 100644 --- a/build.sbt +++ b/build.sbt @@ -248,8 +248,10 @@ def playProject(projectName: String, port: Int, path: Option[String] = None): Pr Universal / javaOptions ++= Seq( "-Dpidfile.path=/dev/null", s"-Dconfig.file=/opt/docker/conf/application.conf", - s"-Dlogger.file=/opt/docker/conf/logback.xml" - ))) + s"-Dlogger.file=/opt/docker/conf/logback.xml", + "-XX:+PrintCommandLineFlags", "-XX:MaxRAMPercentage=50" + )) + ) } def playImageLoaderProject(projectName: String, port: Int, path: Option[String] = None): Project = { From 3178f8d10014af131e2e6a1f7f9591aabb28fec2 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Wed, 14 May 2025 21:13:48 +0100 Subject: [PATCH 25/27] image loader prints java memory settings. --- build.sbt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/build.sbt b/build.sbt index 60d5dc6b8d..94ee6cc769 100644 --- a/build.sbt +++ b/build.sbt @@ -283,6 +283,7 @@ def playImageLoaderProject(projectName: String, port: Int, path: Option[String] Universal / javaOptions ++= Seq( "-Dpidfile.path=/dev/null", s"-Dconfig.file=/opt/docker/conf/application.conf", - s"-Dlogger.file=/opt/docker/conf/logback.xml" - ))) + s"-Dlogger.file=/opt/docker/conf/logback.xml", + "-XX:+PrintCommandLineFlags" + ))) } From ae93f7a2b30139ed84388c7e7158b1f0457b6a42 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 8 Sep 2024 18:02:37 +0100 Subject: [PATCH 26/27] Cloudbuild all artifacts as 1 build. x86 images only. Multi arch is too slow (14 mins) as buildx repeats the apt-get steps. Cloudbuild uses node 24 for Kahuna build. Explicit jdk 11 build in Cloudbuild build. --- cloudbuild.yaml | 64 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 cloudbuild.yaml diff --git a/cloudbuild.yaml b/cloudbuild.yaml new file mode 100644 index 0000000000..c6785559ac --- /dev/null +++ b/cloudbuild.yaml @@ -0,0 +1,64 @@ +options: + machineType: 'N1_HIGHCPU_8' +steps: + - name: 'node:24-alpine' + entrypoint: 'npm' + dir: 'kahuna' + args: [ 'install' ] + - name: 'node:24-alpine' + entrypoint: 'npm' + dir: 'kahuna' + args: [ 'run', 'dist' ] + + - name: 'gcr.io/$PROJECT_ID/scala-sbt:1.6.2-jdk-11' + args: ['docker:publishLocal'] + + - name: 'gcr.io/cloud-builders/docker' + args: ['tag', 'auth:0.1', 'eu.gcr.io/$PROJECT_ID/auth:$BRANCH_NAME'] + - name: 'gcr.io/cloud-builders/docker' + args: ['push', 'eu.gcr.io/$PROJECT_ID/auth:$BRANCH_NAME'] + + - name: 'gcr.io/cloud-builders/docker' + args: ['tag', 'cropper:0.1', 'eu.gcr.io/$PROJECT_ID/cropper:$BRANCH_NAME'] + - name: 'gcr.io/cloud-builders/docker' + args: ['push', 'eu.gcr.io/$PROJECT_ID/cropper:$BRANCH_NAME'] + + - name: 'gcr.io/cloud-builders/docker' + args: ['tag', 'collections:0.1', 'eu.gcr.io/$PROJECT_ID/collections:$BRANCH_NAME'] + - name: 'gcr.io/cloud-builders/docker' + args: ['push', 'eu.gcr.io/$PROJECT_ID/collections:$BRANCH_NAME'] + + - name: 'gcr.io/cloud-builders/docker' + args: ['tag', 'image-loader:0.1', 'eu.gcr.io/$PROJECT_ID/image-loader:$BRANCH_NAME'] + - name: 'gcr.io/cloud-builders/docker' + args: ['push', 'eu.gcr.io/$PROJECT_ID/image-loader:$BRANCH_NAME'] + + - name: 'gcr.io/cloud-builders/docker' + args: ['tag', 'kahuna:0.1', 'eu.gcr.io/$PROJECT_ID/kahuna:$BRANCH_NAME'] + - name: 'gcr.io/cloud-builders/docker' + args: ['push', 'eu.gcr.io/$PROJECT_ID/kahuna:$BRANCH_NAME'] + + - name: 'gcr.io/cloud-builders/docker' + args: ['tag', 'leases:0.1', 'eu.gcr.io/$PROJECT_ID/leases:$BRANCH_NAME'] + - name: 'gcr.io/cloud-builders/docker' + args: ['push', 'eu.gcr.io/$PROJECT_ID/leases:$BRANCH_NAME'] + + - name: 'gcr.io/cloud-builders/docker' + args: ['tag', 'media-api:0.1', 'eu.gcr.io/$PROJECT_ID/media-api:$BRANCH_NAME'] + - name: 'gcr.io/cloud-builders/docker' + args: ['push', 'eu.gcr.io/$PROJECT_ID/media-api:$BRANCH_NAME'] + + - name: 'gcr.io/cloud-builders/docker' + args: ['tag', 'metadata-editor:0.1', 'eu.gcr.io/$PROJECT_ID/metadata-editor:$BRANCH_NAME'] + - name: 'gcr.io/cloud-builders/docker' + args: ['push', 'eu.gcr.io/$PROJECT_ID/metadata-editor:$BRANCH_NAME'] + + - name: 'gcr.io/cloud-builders/docker' + args: ['tag', 'thrall:0.1', 'eu.gcr.io/$PROJECT_ID/thrall:$BRANCH_NAME'] + - name: 'gcr.io/cloud-builders/docker' + args: ['push', 'eu.gcr.io/$PROJECT_ID/thrall:$BRANCH_NAME'] + + - name: 'gcr.io/cloud-builders/docker' + args: ['tag', 'usage:0.1', 'eu.gcr.io/$PROJECT_ID/usage:$BRANCH_NAME'] + - name: 'gcr.io/cloud-builders/docker' + args: ['push', 'eu.gcr.io/$PROJECT_ID/usage:$BRANCH_NAME'] From b1d31d37c729fb6a629f0177e19ccf36ebf07bf7 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Tue, 20 May 2025 18:30:20 +0100 Subject: [PATCH 27/27] Cloudbuild runs Kahuna tests. --- cloudbuild.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cloudbuild.yaml b/cloudbuild.yaml index c6785559ac..596bb770d9 100644 --- a/cloudbuild.yaml +++ b/cloudbuild.yaml @@ -5,6 +5,10 @@ steps: entrypoint: 'npm' dir: 'kahuna' args: [ 'install' ] + - name: 'node:24-alpine' + entrypoint: 'npm' + dir: 'kahuna' + args: [ 'run', 'test' ] - name: 'node:24-alpine' entrypoint: 'npm' dir: 'kahuna'