Skip to content

image/save: Fix layers order in OCI manifest #47160

Merged
thaJeztah merged 2 commits intomoby:masterfrom
vvoland:save-fix-oci-diffids
Jan 22, 2024
Merged

image/save: Fix layers order in OCI manifest #47160
thaJeztah merged 2 commits intomoby:masterfrom
vvoland:save-fix-oci-diffids

Conversation

@vvoland
Copy link
Contributor

@vvoland vvoland commented Jan 22, 2024

Order the layers in OCI manifest by their actual apply order. This is required by the OCI image spec.

- How to verify it

$ docker save nginx:1.25.3 >a.tar
$ ctr image import --platform linux/amd64  a.tar
unpacking docker.io/library/nginx:1.25.3 (sha256:978a1ef6837b075ba0d8209ca4280191bfa28c6bfc08fd526e91303c410a18c0)...done

- Description for the changelog

- Fix layer order in the OCI manifest produced by `docker save`

- A picture of a cute animal (not mandatory but encouraged)

@vvoland
image/save: Change layers type to DiffID
4979605 
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
@vvoland
image/save: Fix layers order in OCI manifest
17fd656 
Order the layers in OCI manifest by their actual apply order. This is
required by the OCI image spec.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
@vvoland vvoland added status/2-code-review area/images Image Distribution kind/bugfix PR's that fix bugs labels Jan 22, 2024
@vvoland vvoland added this to the 26.0.0 milestone Jan 22, 2024
@vvoland vvoland self-assigned this Jan 22, 2024
This was referenced Jan 22, 2024
Closed
Merged
laurazard
laurazard approved these changes Jan 22, 2024
View reviewed changes
Copy link
Member

@laurazard laurazard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@laurazard
Copy link
Member

laurazard commented Jan 22, 2024

We should add some tests for this as well! Can leave that for another PR though, lets get this in.

@thaJeztah
Copy link
Member

thaJeztah commented Jan 22, 2024

We should add some tests for this as well! Can leave that for another PR though, lets get this in.

Yup, agreed; wondering if this can be tested by exporting one of the "frozen images" that we use in CI; those are pinned by digest, so we can use them as fixture.

moby/Dockerfile

Lines 129 to 134 in 42a0788

RUN /download-frozen-image-v2.sh /build \
busybox:latest@sha256:95cf004f559831017cdf4628aaf1bb30133677be8702a8c5f2994629f637a209 \
busybox:glibc@sha256:1f81263701cddf6402afe9f33fca0266d9fff379e59b1748f33d3072da71ee85 \
debian:bookworm-slim@sha256:2bc5c236e9b262645a323e9088dfa3bb1ecb16cc75811daf40a23a824d665be9 \
hello-world:latest@sha256:d58e752213a51785838f9eed2b7a498ffa1cb3aa7f946dda11af39286c3db9a9 \
arm32v7/hello-world:latest@sha256:50b8560ad574c779908da71f7ce370c0a2471c098d44d1c8f6b513c5a55eeeb1

@thaJeztah thaJeztah merged commit b6a5c29 into moby:master Jan 22, 2024
@vvoland
Copy link
Contributor Author

vvoland commented Jan 22, 2024

Yeah we'd just need to add an image that has more than 1 layer :)

@vvoland vvoland mentioned this pull request Jan 22, 2024
Closed
@thaJeztah
Copy link
Member

thaJeztah commented Jan 22, 2024

Yeah we'd just need to add an image that has more than 1 layer :)

DOH! I thought we did, but we removed the buildpacks-dev one in #42763

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@laurazard laurazard laurazard approved these changes

Assignees

@vvoland vvoland

Labels

area/images Image Distribution kind/bugfix PR's that fix bugs process/cherry-picked status/2-code-review

Projects

None yet

Milestone

26.0.0

Development

Successfully merging this pull request may close these issues.

[v25] docker save produces wrong diffIDs

3 participants

@vvoland @laurazard @thaJeztah