Skip to content

Commit 1ef438f

Browse files
committed
fix: fix remote hashing to work with images with no repository
1 parent bd88152 commit 1ef438f

File tree

4 files changed

+23
-4
lines changed

4 files changed

+23
-4
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ The index `IX_METADATA_ENTRY_WEU_CFQN_JSI_JRA_MK` is added to `METADATA_ENTRY`.
2323

2424
This index supports planned metadata API enhancements that enable querying at granular scopes, namely calls, shards, and attempts.
2525

26+
### AWS ECR Docker Remote Hashing
27+
28+
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.
29+
2630
## 91 Release Notes
2731

2832
#### Removal of Google LifeSciences backend code

dockerHashing/src/main/scala/cromwell/docker/DockerImageIdentifier.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ sealed trait DockerImageIdentifier {
1818
lazy val name = repository map { r => s"$r/$image" } getOrElse image
1919
// The name of the image with a repository prefix if a repository was specified, or with a default repository prefix of
2020
// "library" if no repository was specified.
21-
lazy val nameWithDefaultRepository = repository.getOrElse(DefaultRepository) + s"/$image"
21+
lazy val nameWithDefaultRepository = repository.getOrElse(DefaultRepository) + s"$image"
2222
lazy val hostAsString = host map { h => s"$h/" } getOrElse ""
2323
// The full name of this image, including a repository prefix only if a repository was explicitly specified.
2424
lazy val fullName = s"$hostAsString$name:$reference"
@@ -59,7 +59,7 @@ case class DockerImageIdentifierWithHash(host: Option[String],
5959

6060
object DockerImageIdentifier {
6161
private val DefaultDockerTag = "latest"
62-
val DefaultRepository = "library"
62+
val DefaultRepository = ""
6363

6464
private val DockerStringRegex =
6565
s"""

dockerHashing/src/main/scala/cromwell/docker/registryv2/DockerRegistryV2Abstract.scala

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,14 @@ abstract class DockerRegistryV2Abstract(override val config: DockerRegistryConfi
192192
*/
193193
protected def serviceName: Option[String] = None
194194

195+
/**
196+
* Returns the repository path to use in manifest and token URIs.
197+
* Default behavior adds "library" repository for images without an explicit repository.
198+
* Registries can override this for custom behavior (e.g., ECR doesn't need "library").
199+
*/
200+
protected def getRepositoryPath(dockerImageID: DockerImageIdentifier): String =
201+
dockerImageID.nameWithDefaultRepository
202+
195203
/**
196204
* Builds the token URI to be queried based on a DockerImageID
197205
*/
@@ -201,7 +209,7 @@ abstract class DockerRegistryV2Abstract(override val config: DockerRegistryConfi
201209
scheme = Option(Scheme.https),
202210
authority = Option(Authority(host = Uri.RegName(authorizationServerHostName(dockerImageID)))),
203211
path = "/token",
204-
query = Query.fromString(s"${service}scope=repository:${dockerImageID.nameWithDefaultRepository}:pull")
212+
query = Query.fromString(s"${service}scope=repository:${getRepositoryPath(dockerImageID)}:pull")
205213
)
206214
}
207215

@@ -233,7 +241,7 @@ abstract class DockerRegistryV2Abstract(override val config: DockerRegistryConfi
233241
Uri.apply(
234242
scheme = Option(Scheme.https),
235243
authority = Option(Authority(host = Uri.RegName(registryHostName(dockerImageID)))),
236-
path = s"/v2/${dockerImageID.nameWithDefaultRepository}/manifests/${dockerImageID.reference}"
244+
path = s"/v2/${getRepositoryPath(dockerImageID)}/manifests/${dockerImageID.reference}"
237245
)
238246

239247
/**

dockerHashing/src/main/scala/cromwell/docker/registryv2/flows/aws/AmazonEcrAbstract.scala

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,13 @@ import org.slf4j.{Logger, LoggerFactory}
1111
abstract class AmazonEcrAbstract(override val config: DockerRegistryConfig) extends DockerRegistryV2Abstract(config) {
1212
private val logger: Logger = LoggerFactory.getLogger(this.getClass)
1313

14+
/**
15+
* ECR supports images with no repository (e.g., 123456789012.dkr.ecr.us-east-1.amazonaws.com/example-tool)
16+
* so we don't add the default "library" repository prefix
17+
*/
18+
override protected def getRepositoryPath(dockerImageID: DockerImageIdentifier): String =
19+
dockerImageID.name
20+
1421
/**
1522
* Not used as getToken is overridden
1623
*/

0 commit comments

Comments
 (0)