Skip to content

Fix remote hashing to work with images with no repository #7825

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
sberss wants to merge 2 commits into from
Closed

Fix remote hashing to work with images with no repository #7825

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
de2bf9d
fix: fix remote hashing to work with images with no repository
sberss Nov 25, 2025
211c6a5
chore: update changelog
sberss Nov 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ The index `IX_METADATA_ENTRY_WEU_CFQN_JSI_JRA_MK` is added to `METADATA_ENTRY`.

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

### 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.

## 91 Release Notes

#### Removal of Google LifeSciences backend code
Expand Down
4 changes: 2 additions & 2 deletions dockerHashing/src/main/scala/cromwell/docker/DockerImageIdentifier.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"""
Expand Down
12 changes: 10 additions & 2 deletions dockerHashing/src/main/scala/cromwell/docker/registryv2/DockerRegistryV2Abstract.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand All @@ -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")
)
}

Expand Down Expand Up @@ -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}"
)

/**
Expand Down
7 changes: 7 additions & 0 deletions dockerHashing/src/main/scala/cromwell/docker/registryv2/flows/aws/AmazonEcrAbstract.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down