From 6ef03d947019a32df621a8e4564aa3202b16186c Mon Sep 17 00:00:00 2001 From: Liz Baldo Date: Thu, 4 Dec 2025 16:49:00 -0500 Subject: [PATCH 1/2] copy fix over from PR #7825 --- CHANGELOG.md | 5 +++++ .../cromwell/docker/DockerImageIdentifier.scala | 4 ++-- .../docker/registryv2/DockerRegistryV2Abstract.scala | 12 ++++++++++-- .../registryv2/flows/aws/AmazonEcrAbstract.scala | 7 +++++++ 4 files changed, 24 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 249775fb311..f8753f2b7f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## 92 Release Notes +### AWS ECR Docker Remote Hashing +* Fixed an issue where ECR images without an explicit repository prefix +(e.g., `123456789012.dkr.ecr.us-east-1.amazonaws.com/example-tool`) would fail during remote hash computation due to incorrect manifest URI construction. +The Docker registry implementation now correctly handles ECR's support for repository-less image paths. + ### AWS Batch * Moved the option to tag job resources from runtime attributes to backend config. * Appended the custom labels to the list of resource tags to propagate diff --git a/dockerHashing/src/main/scala/cromwell/docker/DockerImageIdentifier.scala b/dockerHashing/src/main/scala/cromwell/docker/DockerImageIdentifier.scala index a20e6e43c5b..fa5aec46dd3 100644 --- a/dockerHashing/src/main/scala/cromwell/docker/DockerImageIdentifier.scala +++ b/dockerHashing/src/main/scala/cromwell/docker/DockerImageIdentifier.scala @@ -18,7 +18,7 @@ sealed trait DockerImageIdentifier { lazy val name = repository map { r => s"$r/$image" } getOrElse image // The name of the image with a repository prefix if a repository was specified, or with a default repository prefix of // "library" if no repository was specified. - lazy val nameWithDefaultRepository = repository.getOrElse(DefaultRepository) + s"/$image" + lazy val nameWithDefaultRepository = repository.getOrElse(DefaultRepository) + s"$image" lazy val hostAsString = host map { h => s"$h/" } getOrElse "" // The full name of this image, including a repository prefix only if a repository was explicitly specified. lazy val fullName = s"$hostAsString$name:$reference" @@ -59,7 +59,7 @@ case class DockerImageIdentifierWithHash(host: Option[String], object DockerImageIdentifier { private val DefaultDockerTag = "latest" - val DefaultRepository = "library" + val DefaultRepository = "" private val DockerStringRegex = s""" diff --git a/dockerHashing/src/main/scala/cromwell/docker/registryv2/DockerRegistryV2Abstract.scala b/dockerHashing/src/main/scala/cromwell/docker/registryv2/DockerRegistryV2Abstract.scala index 6abb41e6551..97298d463af 100644 --- a/dockerHashing/src/main/scala/cromwell/docker/registryv2/DockerRegistryV2Abstract.scala +++ b/dockerHashing/src/main/scala/cromwell/docker/registryv2/DockerRegistryV2Abstract.scala @@ -192,6 +192,14 @@ abstract class DockerRegistryV2Abstract(override val config: DockerRegistryConfi */ protected def serviceName: Option[String] = None + /** + * Returns the repository path to use in manifest and token URIs. + * Default behavior adds "library" repository for images without an explicit repository. + * Registries can override this for custom behavior (e.g., ECR doesn't need "library"). + */ + protected def getRepositoryPath(dockerImageID: DockerImageIdentifier): String = + dockerImageID.nameWithDefaultRepository + /** * Builds the token URI to be queried based on a DockerImageID */ @@ -201,7 +209,7 @@ abstract class DockerRegistryV2Abstract(override val config: DockerRegistryConfi scheme = Option(Scheme.https), authority = Option(Authority(host = Uri.RegName(authorizationServerHostName(dockerImageID)))), path = "/token", - query = Query.fromString(s"${service}scope=repository:${dockerImageID.nameWithDefaultRepository}:pull") + query = Query.fromString(s"${service}scope=repository:${getRepositoryPath(dockerImageID)}:pull") ) } @@ -233,7 +241,7 @@ abstract class DockerRegistryV2Abstract(override val config: DockerRegistryConfi Uri.apply( scheme = Option(Scheme.https), authority = Option(Authority(host = Uri.RegName(registryHostName(dockerImageID)))), - path = s"/v2/${dockerImageID.nameWithDefaultRepository}/manifests/${dockerImageID.reference}" + path = s"/v2/${getRepositoryPath(dockerImageID)}/manifests/${dockerImageID.reference}" ) /** diff --git a/dockerHashing/src/main/scala/cromwell/docker/registryv2/flows/aws/AmazonEcrAbstract.scala b/dockerHashing/src/main/scala/cromwell/docker/registryv2/flows/aws/AmazonEcrAbstract.scala index 4533547ca00..d29f8c7d611 100644 --- a/dockerHashing/src/main/scala/cromwell/docker/registryv2/flows/aws/AmazonEcrAbstract.scala +++ b/dockerHashing/src/main/scala/cromwell/docker/registryv2/flows/aws/AmazonEcrAbstract.scala @@ -11,6 +11,13 @@ import org.slf4j.{Logger, LoggerFactory} abstract class AmazonEcrAbstract(override val config: DockerRegistryConfig) extends DockerRegistryV2Abstract(config) { private val logger: Logger = LoggerFactory.getLogger(this.getClass) + /** + * ECR supports images with no repository (e.g., 123456789012.dkr.ecr.us-east-1.amazonaws.com/example-tool) + * so we don't add the default "library" repository prefix + */ + override protected def getRepositoryPath(dockerImageID: DockerImageIdentifier): String = + dockerImageID.name + /** * Not used as getToken is overridden */ From 48b48b2fa9663d7cc86f2108578e6e237a370e71 Mon Sep 17 00:00:00 2001 From: Liz Baldo Date: Fri, 5 Dec 2025 09:57:37 -0500 Subject: [PATCH 2/2] revert docker image identifier changes --- .../main/scala/cromwell/docker/DockerImageIdentifier.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dockerHashing/src/main/scala/cromwell/docker/DockerImageIdentifier.scala b/dockerHashing/src/main/scala/cromwell/docker/DockerImageIdentifier.scala index fa5aec46dd3..a20e6e43c5b 100644 --- a/dockerHashing/src/main/scala/cromwell/docker/DockerImageIdentifier.scala +++ b/dockerHashing/src/main/scala/cromwell/docker/DockerImageIdentifier.scala @@ -18,7 +18,7 @@ sealed trait DockerImageIdentifier { lazy val name = repository map { r => s"$r/$image" } getOrElse image // The name of the image with a repository prefix if a repository was specified, or with a default repository prefix of // "library" if no repository was specified. - lazy val nameWithDefaultRepository = repository.getOrElse(DefaultRepository) + s"$image" + lazy val nameWithDefaultRepository = repository.getOrElse(DefaultRepository) + s"/$image" lazy val hostAsString = host map { h => s"$h/" } getOrElse "" // The full name of this image, including a repository prefix only if a repository was explicitly specified. lazy val fullName = s"$hostAsString$name:$reference" @@ -59,7 +59,7 @@ case class DockerImageIdentifierWithHash(host: Option[String], object DockerImageIdentifier { private val DefaultDockerTag = "latest" - val DefaultRepository = "" + val DefaultRepository = "library" private val DockerStringRegex = s"""