feat: support pinning images by digest - #948
Conversation
Add the ability to pin a Docker image to an immutable content digest (`sha256:...`) instead of relying solely on a mutable tag, which can be overwritten in the registry. - `Image::digest()` returns an optional digest, letting image implementations ship a default pin. - `ImageExt::with_digest()` overrides the digest per run, taking precedence over the image's own `digest()`. - When a digest resolves, the reference passed to Docker becomes `name:tag@digest`: Docker resolves by digest while the tag is kept for readability. Closes testcontainers#411
✅ Deploy Preview for testcontainers-rust ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
DDtKey
left a comment
DDtKey
left a comment
There was a problem hiding this comment.
Thanks for the contribution!
It looks good to me, though one thing to note and I want to think about: tag remains required with this interface 🤔
But in fact, you can pull an image just by name and digest, like with docker cli:
docker pull <image-name>@sha256:<hash-value>
It's even should currently work if you specify hash as a tag - but it's semantically confusing.
I think we need to consider better interface, but at the same time I'd prefer to avoid breaking changes (or at least go through "deprecation" if it will be needed)
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f28473e4ae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // An explicit `with_digest` override takes precedence over a digest baked into the image. | ||
| // When a digest is present, the reference becomes `name:tag@digest`: Docker resolves the | ||
| // image by digest, while the tag is kept for readability. | ||
| match self.image_digest.as_deref().or_else(|| self.image.digest()) { |
There was a problem hiding this comment.
When an Image implementation supplies a default digest, with_tag(...) and with_name(...) only set the override fields, but this line still falls back to self.image.digest(). That makes MyImage.with_tag("2.0") produce name:2.0@<digest-for-the-default-image>, and because Docker resolves the reference by digest, the caller still runs the pinned default image content rather than the requested tag or repository; this will affect any module that adopts Image::digest() while continuing to support image overrides.
Useful? React with 👍 / 👎.
|
Actually, I just realized that it is already possible to pin by digest using the existing API. The following will fail: // docker pull bitnamisecure/redis:@sha256:d842c434ff617b84f954700b60fd99ac8b567ce16292daccb18cfc214cdcc2ec
// Error: invalid reference format
GenericImage::new(
"bitnamisecure/redis",
"@sha256:d842c434ff617b84f954700b60fd99ac8b567ce16292daccb18cfc214cdcc2ec"
)But including a tag (that will be ignored) works for the same reason that these changes do: // docker pull bitnamisecure/redis:latest@sha256:d842c434ff617b84f954700b60fd99ac8b567ce16292daccb18cfc214cdcc2ec
GenericImage::new(
"bitnamisecure/redis",
"latest@sha256:d842c434ff617b84f954700b60fd99ac8b567ce16292daccb18cfc214cdcc2ec"
)It is a bit of a hack - but it will essentially do the exact same thing as my PR, so this PR doesn't actually add any new functionality. The format |
darth-raijin
left a comment
darth-raijin
left a comment
There was a problem hiding this comment.
I think DDtKey raises a good point about the fact that Image::tag() is mandatory in the Testcontainers API, while Docker's image-reference model does not require a tag when an image is selected by digest.
Docker supports digest-only references in the form:
<image-name>@sha256:<digest>
The common cases are:
name + tag
or:
name + digest
The Docker API also accepts references containing both a tag and a digest. In that form, the digest is the authoritative identifier used to select the image.
Modelling tag and digest as two independent optional properties introduces additional state and raises questions about how the two values should interact when both are present.
Tags are already part of the public Image trait and are implemented throughout the community modules, for example in the Postgres module. Changing tag() to return Option<&str>, removing it, or introducing another required trait method would therefore be a breaking change.
However, internally, Image::tag() appears to have only one direct production consumer: ContainerRequest::descriptor(). The resulting descriptor is then used by the Docker integration for image pulling and container creation.
This makes ContainerRequest::descriptor() a relatively narrow translation point between the public Image API and the final Docker image reference.
Instead of adding digest() as another independent property alongside tag(), could we introduce a typed representation of how the image is selected?
For instance:
pub enum ImageSelector {
Tag(String),
Digest(String),
}
pub trait Image {
fn name(&self) -> &str;
fn tag(&self) -> &str;
fn image_selector(&self) -> ImageSelector {
ImageSelector::Tag(self.tag().to_owned())
}
}Existing implementations would remain source-compatible because the default implementation preserves the current behaviour where images are referenced through their tags.
An image that should be resolved by digest could override the new method:
fn image_selector(&self) -> ImageSelector {
ImageSelector::Digest(self.digest.clone())
}The core idea would be for ContainerRequest::descriptor() to treat image_selector() as the source of truth:
match image_selector {
ImageSelector::Tag(tag) => format!("{name}:{tag}"),
ImageSelector::Digest(digest) => format!("{name}@{digest}"),
}There is still one limitation to discuss: Image::tag() remains a required public trait method, a digest-based Image implementation must continue to provide it even if descriptor resolution uses image_selector().
This leaves some temporary semantic awkwardness in the public trait, but preserves compatibility and provides an incremental migration path toward making image_selector() the canonical representation in a future breaking release.
|
Tick the box to add this pull request to the merge queue (same as
|
3 participants
Add the ability to pin a Docker image to an immutable content digest (
sha256:...) instead of relying solely on a mutable tag, which can be overwritten in the registry.Image::digest()returns an optional digest, letting image implementations ship a default pin.ImageExt::with_digest()overrides the digest per run, taking precedence over the image's owndigest().name:tag@digest: Docker resolves by digest while the tag is kept for readability.Closes #411