[sha512] image/copy: allow users to force digest algorithm in CLI (e.g. `skopeo copy --force-digest`) #552

lsm5 · 2025-12-10T21:12:02Z

Depends on #512 . Only see HEAD commit here.

The goal is to allow building the digest-choice-dependent machinery from the bottom up without committing to an API while we don't understand the problem space; for now, we don't expose any way for users to actually make a choice. This code is never called at this point, so this should not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>

for now, purely to let us internally work on implementing digest choice / conversions; there is no way for a caller to set it. Should not change behavior, the field has no users. Signed-off-by: Miloslav Trmač <mitr@redhat.com>

Signed-off-by: Miloslav Trmač <mitr@redhat.com>

This is: - UNUSABLE (no external API) - INCOMPLETE (manifest digests, TryReusingBlob, ...) - BROKEN (various code assumes that digest returned from upload matches input) - INCONSISTENT (PutBlobOptions.Digests is only implemented in one transport, and silently ignored elsewhere) Signed-off-by: Miloslav Trmač <mitr@redhat.com>

…eo copy Changes to image/copy/blob.go: - Use digests.Options.Choose() to validate digest changes Changes to image/copy/copy.go: - Add SetForceDigestAlgorithm() helper to configure digest forcing Changes to image/copy/single.go: - Add updateManifestConfigDigest() helper that uses typed manifest structures (OCI1/Schema2) to update config digest, avoiding generic JSON manipulation - Modify copyConfig() to return the new config digest when changed - Update copyUpdatedConfigAndManifest() to handle config digest changes by calling updateManifestConfigDigest() when needed Signed-off-by: Lokesh Mandvekar <lsm5@redhat.com>

podmanbot · 2025-12-10T21:13:18Z

✅ A new PR has been created in buildah to vendor these changes: containers/buildah#6597

mtrmac

Thanks, this is valuable.

On the process side, I’m not quite sure how to go about building this — I’d like to avoid exposing and API that plain doesn’t work for some transports. I think I’d prefer:

Building this ~one complete feature at a time, e.g. if we add PutBlobOptions.Digests, then actually implement it in all transports immediately. We should never be failing to enforce ForceDigest… and reporting success. (In many cases, a Situation{CannotChangeDigestReason: "a sha256-only transport not updated yet", digest: sha256Value} in a transport would be fine, that’s what the tracker is for, as long as the enforcement of the rules is all there immediately.)
Not providing the public API until all pieces are in place. If we need something for testing, a temporary BrokenSetForceDigestAlgorithm /* Unstable API, may be changed or removed any time */?

Without a deep review or revisiting earlier PRs, I think missing pieces here are

multi-platform images: should not be too difficult, but is fairly orthogonal
TryReusingBlob: this + BlobInfoCache will be significant, it has ~the same prerequisite question “what layer reuse is valid cross-digest” as c/storage — and that’s a major reason why I’d prefer to build this a feature at a time, so that we can get plain PutBlob or configs done without blocking on this
The PutBlobPartial code path (plausibly should be just a failure because pull --force-digest= makes little sense, and would be difficult to implement, I’m not sure)
DiffID computation (maybe keep sha256 and fail on schema1??)
ZstdChunked compression using sha512 digests for all its data

mtrmac · 2025-12-11T18:06:21Z

image/copy/blob.go

 	}
 	if stream.info.Digest != "" && uploadedInfo.Digest != stream.info.Digest {
-		return types.BlobInfo{}, fmt.Errorf("Internal error writing blob %s, blob with digest %s saved with digest %s", srcInfo.Digest, stream.info.Digest, uploadedInfo.Digest)
+		expectedAlgo, err := ic.c.options.digestOptions.Choose(digests.Situation{


This is redundant / conflicting with PutBlob… making its own choice. If we had the generic code choosing a specific algorithm and expecting precisely that one to be used, then it should do that before PutBlob and provide that to transports as a parameter.

I’m not sure where the decision should be, at this point [and it’s a private interface, so we can change our mind] — I guess the transport might have some information (e.g. about remote registry capabilities) that the generic code doesn’t — but, for now, we don’t have all transports updated yet, either way…

AFAICS this only exists as a sanity check (both config and layer copies later enforce equality if cannotModifyManifest…), so I think an “if algorithms match the whole values must match” check here would be sufficient.

mtrmac · 2025-12-11T18:08:51Z

image/copy/copy.go

+}
+
+// SetForceDigestAlgorithm forces the use of a specific digest algorithm for this copy operation.
+func (o *Options) SetForceDigestAlgorithm(algo digest.Algorithm) error {


(Yes, I’m generally in favor of adding more options via methods rather than fields. And we can always add something vaguely like SetForceDigestOptions later if we decided to make that fully public.)

I’m wondering about naming this ForceDestinationDigest… — we might eventually want a “pulls must consume sha512 or stronger digests and refuse to work on sha256”, and that’s a fairly unrelated feature.

mtrmac · 2025-12-11T18:09:45Z

image/copy/copy.go

+	if err != nil {
+		return fmt.Errorf("failed to set force-digest algorithm: %w", err)
+	}
+	o.digestOptions = digestOpts


Fail if this is already set? It might protect against oversights. OTOH it can also prevent some code patterns — if that turned out to be troublesome, we can always remove that enforcement later.

mtrmac · 2025-12-11T18:14:31Z

image/copy/copy.go

+	// only to allow gradually building the feature set.
+	// After c/image/copy consistently implements it, provide a public digest options API of some kind.
+	optionsCopy := *options
+	// Only set default if not already configured


(We can avoid the whole optionsCopy if digestOptions != nil.)

mtrmac · 2025-12-11T18:19:11Z

image/copy/single.go

 		if destInfo.Digest != srcInfo.Digest {
-			return fmt.Errorf("Internal error: copying uncompressed config blob %s changed digest to %s", srcInfo.Digest, destInfo.Digest)
+			// Allow digest algorithm changes when forcing a specific digest algorithm
+			forcingDifferentAlgo := ic.c.options.digestOptions.MustUseSet() != "" &&


See the similar check copyBlobFromStream

“MustUse” Is not the only reason a copy might choose a different algorithm (e.g. we might be pushing to a sha256-only-transport or registry), hard-coding a check only for MustUse here is overly specific

Compare the end of copyLayers, where the equality check is immediately paired with manifest edits [and see elsewhere about manifest edits], so that we always set up edits if we know they are needed, and we minimize the risk of the two getting out of sync. I’m not sure whether the check+update should live in copyConfig or copyUpdatedConfigAndManifest (guessing the latter), but they should be together.

mtrmac · 2025-12-11T18:24:32Z

image/copy/single.go


+// updateManifestConfigDigest uses typed manifest structures instead of generic JSON manipulation.
+// This leverages the existing manifest parsing and serialization infrastructure.
+func (ic *imageCopier) updateManifestConfigDigest(manifestBlob []byte, src types.Image, newConfigDigest digest.Digest) ([]byte, error) {


This needs to go through the manifest abstraction in c/image/internal/image and not be hard-coded in c/image/copy.

(It can be a method on image.Sourced/genericManifest, like CanChangeLayerCompression, without making it a public API, at least for now.)

mtrmac · 2025-12-11T18:25:31Z

image/copy/single.go

+	}
+
 	ic.c.Printf("Writing manifest to image destination\n")
 	manifestDigest, err := manifest.Digest(man)


(This is another part that will need updating, for multi-arch images.)

mtrmac · 2025-12-11T20:15:35Z

FWIW containers/buildah#6597 hits internal error: use of uninitialized digests.Options — entirely my fault, it’s already broken in #512, but, still a blocker to merging this as well.

mtrmac · 2025-12-11T22:04:52Z

FWIW containers/buildah#6597 hits internal error: use of uninitialized digests.Options

I think #512 has a fix, but I’m not going to stay around today to wait for containers/buildah#6552 to confirm.

lsm5 · 2025-12-12T15:30:40Z

I think I’d prefer:
* Not providing the public API until all pieces are in place. If we need something for testing, a temporary BrokenSetForceDigestAlgorithm /* Unstable API, may be changed or removed any time */?

I can go ahead with this. Let me know if you'd much rather the other preference though.

mtrmac · 2025-12-12T20:59:33Z

I didn’t mean the two bullet points as alternatives, but as two parts of the same plan.

My main concern here is having some way of tracking what doesn’t work yet; all of that will need to either be implemented or at least be updated to cleanly fail. So I was thinking “one feature at a time, in all transports”. Doing it that way also means we immediately run into difficulties if some transports’ structures make a design difficult.

There may be other ways to track things or to structure the implementation. And maybe that whole discussion is premature, and we need a prototype PR, or a few, sort of like this, testing the full workflow to just run into things and see what are the things that will need to be done.

mtrmac and others added 5 commits December 10, 2025 14:24

Add image.copy.digestOptions

b10f5a6

for now, purely to let us internally work on implementing digest choice / conversions; there is no way for a caller to set it. Should not change behavior, the field has no users. Signed-off-by: Miloslav Trmač <mitr@redhat.com>

Add DigestIfAlgorithmUnknown

d6e3e04

Signed-off-by: Miloslav Trmač <mitr@redhat.com>

github-actions bot added the image Related to "image" package label Dec 10, 2025

lsm5 mentioned this pull request Dec 10, 2025

[sha512] cmd/skopeo/copy: New --force-digest flag containers/skopeo#2771

Draft

podmanbot pushed a commit to podmanbot/buildah that referenced this pull request Dec 10, 2025

dnm: Vendor changes from containers/container-libs#552

db8df69

podmanbot mentioned this pull request Dec 10, 2025

Sync: [sha512] image/copy: allow users to force digest algorithm in CLI (e.g. skopeo copy --force-digest) containers/buildah#6597

Draft

mtrmac reviewed Dec 11, 2025

View reviewed changes

[sha512] image/copy: allow users to force digest algorithm in CLI (e.g. skopeo copy --force-digest) #552

Are you sure you want to change the base?

[sha512] image/copy: allow users to force digest algorithm in CLI (e.g. skopeo copy --force-digest) #552

Uh oh!

Conversation

lsm5 commented Dec 10, 2025

Uh oh!

podmanbot commented Dec 10, 2025

Uh oh!

mtrmac left a comment

Choose a reason for hiding this comment

Uh oh!

mtrmac Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

mtrmac Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

mtrmac Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

mtrmac Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

mtrmac Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

mtrmac Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

mtrmac Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

mtrmac commented Dec 11, 2025

Uh oh!

mtrmac commented Dec 11, 2025

Uh oh!

lsm5 commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtrmac commented Dec 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[sha512] image/copy: allow users to force digest algorithm in CLI (e.g. `skopeo copy --force-digest`) #552

[sha512] image/copy: allow users to force digest algorithm in CLI (e.g. `skopeo copy --force-digest`) #552

lsm5 commented Dec 12, 2025 •

edited

Loading