Skip to content
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.

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

container: Add application layer to the correct end of the layer stack #51

Merged
merged 2 commits into from
Jan 30, 2025

Conversation

euanh
Copy link
Collaborator

@euanh euanh commented Jan 27, 2025

Motivation

containertool currently adds the app layer to the beginning of the
layer stack array in the manifest. This results in the app layer
being the first to be unpacked, with the others stacked on top. We
can show this by adding a plain text file as the executable. If we
stack another layer on top with a file of the same name, it should
replace the underlying one but it does not:

echo first > bar
swift run containertool --repository localhost:5555/bar bar
podman run --pull=always -it --rm --entrypoint=cat localhost:5555/bar:latest bar
# prints: first

echo second > bar
swift run containertool --repository localhost:5555/bar bar --from localhost:5555/bar:latest
podman run --pull=always -it --rm --entrypoint=cat localhost:5555/bar:latest bar
# prints: first
# should print: second

Currently containertool is only used to add the application binary
to the application layer. This bug will only cause a problem if the
base layer adds a binary at the same path, because this will override
the application.

This bug probably arose because the specification for the rootfs.diff_ids
field of the image configuration defines the layers as being "in
order from first to last", which could be read ambiguously:
https://github.com/opencontainers/image-spec/blob/main/config.md?plain=1#L220-L222

The specification for the manifest.layers field is much more explicit
about the ordering:
https://github.com/opencontainers/image-spec/blob/fbb4662eb53b80bd38f7597406cf1211317768f0/manifest.md?plain=1#L70-L71

Modifications

Append the application layer to layer stacks in the manifest and configuration blobs, instead of prepending.

Result

This with this change, the second build and container run in the
example above prints "second" as expected.

Test Plan

This PR adds a new integration test which uses containertool to build two layers and check that they override each other correctly.

All existing tests continue to pass.

Fixes #57

@euanh euanh force-pushed the application-layer-ordering branch 7 times, most recently from 5a22a6b to 5e1c2ab Compare January 27, 2025 12:02
@euanh euanh added kind/bug Something isn't working area/testing Improvements to tests. area/interoperability Improvements to compatibility with other systems. semver/patch No public API change. labels Jan 27, 2025
@euanh euanh force-pushed the application-layer-ordering branch 3 times, most recently from 18b60a4 to 326e007 Compare January 27, 2025 12:42
@euanh euanh linked an issue Jan 30, 2025 that may be closed by this pull request
@euanh euanh assigned euanh and unassigned euanh Jan 30, 2025
@euanh euanh force-pushed the application-layer-ordering branch from 326e007 to bd0e66f Compare January 30, 2025 09:01
@euanh euanh changed the title wip: Test layering container: Add application layer to the correct end of the layer stack Jan 30, 2025
@euanh euanh marked this pull request as ready for review January 30, 2025 09:08
euanh added 2 commits January 30, 2025 09:22
`containertool` currently prepends the app layer to the layer stack,
but it should append it.   This means that a later layer will not
override a file with the same path in an earlier layer.

This commit adds an integration tests which stacks two layers on
top of each other using containertool and verifies that the second
layer overrides the first.
containertool currently adds the app layer to the beginning of the
layer stack array in the manifest. This results in the app layer
being the first to be unpacked, with the others stacked on top. We
can show this by adding a plain text file as the executable. If we
stack another layer on top with a file of the same name, it should
replace the underlying one but it does not:

    echo first > bar
    swift run containertool --repository localhost:5555/bar bar
    podman run --pull=always -it --rm --entrypoint=cat localhost:5555/bar:latest bar
    # prints: first

    echo second > bar
    swift run containertool --repository localhost:5555/bar bar --from localhost:5555/bar:latest
    podman run --pull=always -it --rm --entrypoint=cat localhost:5555/bar:latest bar
    # prints: first
    # should print: second

Currently containertool is only used to add the application binary
to the application layer. This bug will only cause a problem if the
base layer adds a binary at the same path, because this will override
the application.

This bug probably arose because the specification for the rootfs.diff_ids
field of the image configuration defines the layers as being "in
order from first to last", which could be read ambiguously:
https://github.com/opencontainers/image-spec/blob/main/config.md?plain=1#L220-L222

The specification for the manifest.layers field is much more explicit
about the ordering:
https://github.com/opencontainers/image-spec/blob/fbb4662eb53b80bd38f7597406cf1211317768f0/manifest.md?plain=1#L70-L71

This with this change, the second build and container run in the
example above prints "second" as expected.
@euanh euanh force-pushed the application-layer-ordering branch from bd0e66f to 322dd0b Compare January 30, 2025 09:24
@euanh euanh merged commit 40999fd into apple:main Jan 30, 2025
18 checks passed
@euanh euanh deleted the application-layer-ordering branch January 30, 2025 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/interoperability Improvements to compatibility with other systems. area/testing Improvements to tests. kind/bug Something isn't working semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

containertool adds application to the wrong end of the layer stack
1 participant