-
Notifications
You must be signed in to change notification settings - Fork 393
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
Expect UncompressedDigest to be set for partial pulls, enforce DiffID match #2613
Conversation
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
090e94d
to
b859a03
Compare
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
e46c8d0
to
eb0db7b
Compare
> go mod edit -replace github.com/containers/image/v5=github.com/mtrmac/image/v5@wip-authentic Signed-off-by: Miloslav Trmač <mitr@redhat.com>
95cdcf3
to
57b0637
Compare
> go mod edit -replace github.com/containers/image/v5=github.com/mtrmac/image/v5@wip-authentic Signed-off-by: Miloslav Trmač <mitr@redhat.com>
137b760
to
4fb4df8
Compare
> go mod edit -replace github.com/containers/image/v5=github.com/mtrmac/image/v5@wip-authentic Signed-off-by: Miloslav Trmač <mitr@redhat.com>
> go mod edit -replace github.com/containers/image/v5=github.com/mtrmac/image/v5@wip-authentic Signed-off-by: Miloslav Trmač <mitr@redhat.com>
4fb4df8
to
7987093
Compare
@giuseppe RFC. I still need to address / review some corner cases, but I think the broad outline is settled now, and Podman tests are passing. |
d7fdde4
to
c1036a6
Compare
c1036a6
to
290bc1e
Compare
@giuseppe PTAL for an early review. This is mostly untested, but it should be feature-complete and comprehensive. Contrary to the original plan for containers/storage#2180 , this minimizes the impact on |
LGTM, though given my unfamiliarity with the codebase a review from someone else would be much more valuable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
> go mod edit -replace github.com/containers/image/v5=github.com/mtrmac/image/v5@wip-authentic Signed-off-by: Miloslav Trmač <mitr@redhat.com>
LGTM |
Great work @mtrmac ! |
reused.Digest is not always blobDigest, it might be uncompressedDigest; but we must have a blobDiffIDs entry for reused.Digest. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
... because we will start enforcing that the DiffID values match. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
We will use the trustedLayerIdentityData for other purposes in the caller as well. Should not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Keep the commit queuing logic together, this is more of an implementation detail of commitLayer. Only moves unchanged code, should not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
It's fairly isolated from the rest of the function, and if split, it can have unit tests. Those tests are valuable to ensure that layer IDs continue to behave the expected way and maximize layer reuse (although we are not making an API commitment to layer ID values). Should not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Should not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
... to simplify some of the repetitive logging code. Should not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
untrustedLayerDiffID currently specializes the "not available yet" case; also specialize the "image does not provide this at all" case, which we will need to handle. Should not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Two different locations in the function need the data, and the caller must have it available; so always passing it in simplifies the implementation and removes an impossible error path. This might hypothetically make layer reuse a bit worse, if we happened to learn something for trustedLayerIdentityData from processing other layers of the same image, but reusing the same layer twice within an image should be rare. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
…ema1 images Should not change behavior; we call GetTOCDigest in copy.imageCopier.copyLayer before reaching PutBlobPartial, so the new error path should not be reachable. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
…ID values If a layer has a TOC, require that it must have a DiffID commitment, or refuse to pull it partially. Layers without a TOC continue to be allowed to use the partial pull code path, and we don't even require config's RootFS.DiffID to be present. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Remove some completely redundant comments to shorten the code, clarify where appropriate. Should not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
- If a layer has a TOC digest (i.e. could possibly be pulled partially), and c/storage has computed the uncompressed digest, require that the config's RootFS.DiffIDs exists and matches. This fixes the "view ambiguity" of partially-pulled layers. - For _all_ layers, if RootFS.DiffIDs exists and we know the layer's uncompressed digest, also require the RootFS.DiffIDs value to match. This might be a compatibility break, but Docker requires these values anyway. - We happen to allow setting DiffIDs to empty values, if the layer does not have a TOC digest (so there is no risk of "view ambiguity"). Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This resolves the "signing ambiguity" by requiring that images must have a DiffID entry, and it must match, in partial pulls. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This resolves the "signing ambiguity" by requiring that images must have a DiffID entry, and it must match, in partial pulls. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This resolves the "signing ambiguity" by requiring that images must have a DiffID entry, and it must match, in partial pulls. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This resolves the "signing ambiguity" by requiring that images must have a DiffID entry, and it must match, in partial pulls. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Ahmed Moalla (2): add support to `;` for comments in unit files as per systemd documentation Fix unescaping octal escape sequence in values of Quadlet unit files Andrew Sayers (1): Fix podman-restart.service when there are no containers Arthur Sengileyev (3): Fix compilation issues in QEMU machine files (Windows platform) Cover Unix socket in inpect test on Windows platform Improve platform specific URL handling in `podman compose` for machines Ashley Cui (1): Add BuildOrigin field to podman info Brent Baude (11): Add man pages to Mac installer Add newer c/i to support artifacts podman artifact error with libkrun on intel-based machines Remove usused Kind() function Remove unnecessary error handling Prevent two podman machines running on darwin Move detection of libkrun and intel Add type and annotations to artifact add Add --no-trunc to artifact ls Add --noheading to artifact ls Christoph Reiter (1): bin/docker: fix broken escaping and variable substitution Christophe Fergeau (2): gvproxy: Use 0.8.1 binary vfkit: Use 0.6.0 binary Daniel J Walsh (8): AdditionalSupport for SubPath volume mounts When generating host volumes for k8s, force to lowercase Kube volumes can not container _ Document .build for Image .container option Add --no-hostname option Switch all calls of assert.Nil to assert.NoError Replace instances of PodmanExitCleanly in play_kube_test.go Add podman manifest rm --ignore Ed Santiago (11): test f41 VMs Revert "cirrus: test only on f40/rawhide" Reapply "CI: test nftables driver on fedora" kube SIGINT system test: fix race in timeout handling Yet another bump, f41 with fixed kernel Revert "Reapply "CI: test nftables driver on fedora"" Buildah treadmill tweaks system tests: safer install_kube_template() CI: system tests: parallelize 010 Bump CI VMs CI: --image-volume test: robustify Eduardo Santiago (1): make remotesystem: fail early if serial tests fail Erik Sjölund (2): specgenutil: Fix parsing of mount option ptmxmode docs: Add quadlet debug method systemd-analyze Evstifeev Roman (1): docs: mount.md - idmapped mounts only work for root user Federico Di Pierro (1): fix(pkg/rootless): avoid memleak during init() contructor. Florian Apolloner (1): Set network ID if available during container inspect Gavin Lam (2): Add nohosts option to /build and /libpod/build Add --hosts-file flag to container and pod commands George Joseph (1): Pass container hostname to netavark Giuseppe Scrivano (13): test: add zstd:chunked system tests spec: clamp rlimits in a userns vendor: bump containers/buildah libpod: pass down NoPivotRoot to Buildah spec: clamp rlimits without CAP_SYS_RESOURCE stats: ignore errors from containers without cgroups systemd: simplify parser and fix infinite loop test: enable newly added test specgen: fix comment namespaces: allow configuring keep-id userns size util: replace Walk with WalkDir pkg/api: honor cdi devices from the hostconfig rootless: fix hang on s390x Graceson Aufderheide (1): fix podman machine init --ignition-path Gunjan Vyas (4): build: update gvisor-tap-vsock to 0.8.0 gvproxy: Disable port-forwarding on WSL winmake.ps1: Fix the syntax of the function call Win-SSHProxy wsl-e2e: Add a test to ensure port 2222 is free with usermode networking H Dub (2): docs: Enhance podman build --secret documentation and add examples Makefile: Add validatepr description for 'make help' output Jake Correnti (1): Add `machine init --playbook` James Hewitt (2): Add a test for forcing compression and v2s2 format Switch to fixed common Jan Rodák (5): Configure HealthCheck with `podman update` Fix overwriting of LinuxResources structure in the database Fix device limitations in podman-remote update on remote systems Clean up after unexpectedly terminated build Bump FreeBSD version to 13.4 Jindrich Novy (2): Package podman-machine on supported architectures only. Replace ExclusiveArch with ifarch Kashiwa (1): refactor: simplify LinuxNS type definition and String method Leo Liu (2): Update description for completion Remove `.exe` suffix if any Lokesh Mandvekar (9): [CI:ALL] Bump main to v5.4.0-dev [skip-ci] Packit: remove epel and re-enable c9s [skip-ci] Packit/copr: switch to fedora-all system-tests: switch ls with getfattr for selinux tests RPM: adjust qemu dependencies RPM: include empty check to silence rpmlint RPM: cleanup macro defs RPM: set buildOrigin in LDFLAG Update rpm/podman.spec Mario Loriedo (17): Exclude symlink from pre-commit end-of-file-fixer Avoid printing PR text to stdout in system test Update codespell to v2.3.0 New `system connection add` tests Switch to non-installing WSL by default Windows: don't install WSL/HyperV on update Update windows installer tests Fix `podman info` with multiple imagestores Bump WiX toolset version to 5.0.2 Add win installer patch Avoid rebooting on Windows when upgrading and WSL isn't installed Avoid rebooting twice when installing WSL Revert "win-installer test: revert to v5.3.0" Stop creating a patch for v5.3.1 upgrades on windows Avoid upgrading from v5.3.1 on Windows Safer use of `filepath.EvalSymlinks()` on Windows Force use of iptables on Windows WSL Matt Heon (16): Add subpath support to volumes in `--mount` option Update release notes on main for v5.3.0 Overlay mounts supersede image volumes & volumes-from Revert "libpod: remove shutdown.Unregister()" Remove JSON tag from UseImageHosts in ContainerConfig Bump to v5.4.0-rc1 Bump to v5.4.0-dev Bump to v5.4.0-rc2 Bump to v5.4.0-dev Update release notes for v5.4.0-rc3 Bump to v5.4.0-rc3 Bump to v5.4.0-dev Set Cirrus DEST_BRANCH appropriately to fix CI In SQLite state, use defaults for empty-string checks Update release notes for v5.4.0 final Bump to v5.4.0 Matthew Heon (2): Mount volumes before copying into a container Update release notes for v5.4.0-rc2 Maël Azimi (1): doc: fix words repetitions Michael Zimmermann (5): vendor: update containers/common add support for driver-specific options during container creation vendor: update containers/common docs: document bridge mode option docs: improve documentation for internal networks Miloslav Trmač (17): Fix apparent typos in zstd:chunked tests Sanity-check that the test is really using partial pulls Clarify the reason for skip_if_remote Introduce PodmanTestIntegration.PodmanExitCleanly Use PodmanExitCleanly in attach_test.go Turn PodmanAsUserBase into PodmanExecBaseWithOptions Pass all of PodmanExecOptions to various [mM]akeOptions functions Inline PodmanBase into callers Restructure use of options Introduce PodmanTestIntegration.PodmanWithOptions Eliminate PodmanExtraFiles Update expected errors when pulling encrypted images Update c/image after containers/image#2613 Revert "Use the config digest to compare images loaded/pulled using different methods" Fix image ID query Eliminate PodmanSystemdScope Define, and use, PodmanExitCleanlyWithOptions Misaki Kasumi (1): quadlet: fix inter-dependency of containers in `Network=` Nalin Dahyabhai (2): Fix panic in `manifest annotate --index` manifest annotate: connect IndexAnnotations Nicola Sella (1): Use latest version of VS BuildTools Odilon Sousa (1): Add support to ShmSize in Pods with Quadlet Paul Holzinger (41): volume ls: fix race that caused it to fail vendor latest c/{buildah,common,image,storage} test/system: add regression test for TZDIR local issue test/buildah-bud: build new inet helper pkg/machine/e2e: remove dead code update golangci-lint to v1.62.0 vendor containers projects to tagged versions test/e2e: remove FIPS test connection: ignore errors when parsing ssh_config ssh_config: do not overwrite values from config file ssh_config: allow IdentityFile file with tilde only read ssh_config for non machine connections libpod: addHosts() prevent nil deref docs: add 5.3 as Reference version win-installer test: revert to v5.3.0 OWNERS: remove edsantiago Update VM images test/e2e: remove outdated SkipOnOSVersion() calls test/e2e: SkipOnOSVersion() add reason field shell completion: respect CONTAINERS_REGISTRIES_CONF test/system: remove system dial-stdio test test/system: CopyDirectory() do not chown files test/system: fix "podman play --build private registry" error vendor latest c/common from main update golangci/golangci-lint to v1.63.4 New VM Images pkg/machine/e2e: improve "list machine from all providers" pkg/machine/e2e: improve podman.exe match cirrus: bump macos machine test timeout vendor latest c/{common,image,storage} do not set the CreateCommand for API users vendor latest c/{buildah,common,image,storage} test/buildah-bud: skip two new problematic tests on remote libpod: remove unused ExecStartAndAttach() podman exec: correctly support detaching update gvproxy version rpm: add attr as dependency for podman-tests test/e2e: improve write/removeConf() artifact: only allow single manifest Makefile: escape BUILD_ORIGIN properly docs: add v5.4 to API reference Riccardo Paolo Bestetti (1): docs: add 'initialized' state to status filters Robert Günzler (2): Add kube play support for CDI resource allocation Document kube-play CDI support SEIAROTg (1): Fixes missing binary in systemd. Sainath Sativar (1): Log network creation and removal events in Podman Sergio Lopez (1): Bump bundled krunkit to 0.1.4 Simon Westersund (1): Fix slirp4netns typo in podman-network.1.md Tigran Sogomonian (4): api: Replace close function in condition body api: Add error check api: Error checking before NULL dereference api: replace inspectID with name Valentin Rothberg (1): compose docs: fix typo Valery Masiutsin (1): Fixing ~/.ssh/identity handling Warren Young (1): Avoid indirect links through quadlet(5) Ygal Blum (2): Quadlet - Use = sign when setting the pull arg for build Quadlet - make sure the /etc/containers/systemd/users is traversed in rootless jmozmoz (1): Add hint to restart Podman machine to really accept new certificates ksw2000 (2): refact: EventerType and improve consistency refact: use uptime.minutes instead of uptime.seconds renovate[bot] (51): fix(deps): update module golang.org/x/crypto to v0.29.0 fix(deps): update module golang.org/x/tools to v0.27.0 fix(deps): update module golang.org/x/net to v0.31.0 chore(deps): update dependency setuptools to ~=75.4.0 fix(deps): update module github.com/moby/sys/capability to v0.4.0 chore(deps): update dependency setuptools to ~=75.5.0 fix(deps): update module google.golang.org/protobuf to v1.35.2 fix(deps): update module github.com/opencontainers/runc to v1.2.2 fix(deps): update github.com/containers/buildah digest to 52437ef chore(deps): update dependency setuptools to ~=75.6.0 fix(deps): update module github.com/onsi/ginkgo/v2 to v2.22.0 fix(deps): update module github.com/crc-org/crc/v2 to v2.44.0 fix(deps): update module github.com/stretchr/testify to v1.10.0 fix(deps): update github.com/containers/common digest to ceceb40 fix(deps): update module github.com/onsi/gomega to v1.36.0 chore(deps): update dependency golangci/golangci-lint to v1.62.2 fix(deps): update module github.com/crc-org/vfkit to v0.6.0 fix(deps): update github.com/godbus/dbus/v5 digest to c266b19 fix(deps): update golang.org/x/exp digest to 2d47ceb fix(deps): update module github.com/shirou/gopsutil/v4 to v4.24.11 fix(deps): update module github.com/containers/gvisor-tap-vsock to v0.8.1 fix(deps): update github.com/opencontainers/runtime-tools digest to f7e3563 fix(deps): update module golang.org/x/sys to v0.28.0 fix(deps): update module golang.org/x/crypto to v0.30.0 fix(deps): update module golang.org/x/tools to v0.28.0 fix(deps): update module golang.org/x/net to v0.32.0 fix(deps): update module github.com/cyphar/filepath-securejoin to v0.3.5 fix(deps): update module github.com/docker/docker to v27.4.0+incompatible fix(deps): update module github.com/onsi/gomega to v1.36.1 fix(deps): update module github.com/opencontainers/runc to v1.2.3 fix(deps): update module github.com/crc-org/crc/v2 to v2.45.0 fix(deps): update module golang.org/x/crypto to v0.31.0 [security] fix(deps): update module github.com/cpuguy83/go-md2man/v2 to v2.0.6 fix(deps): update module github.com/docker/docker to v27.4.1+incompatible fix(deps): update module golang.org/x/net to v0.33.0 [security] chore(deps): update module golang.org/x/crypto to v0.31.0 [security] fix(deps): update module github.com/onsi/ginkgo/v2 to v2.22.1 fix(deps): update module github.com/moby/term to v0.5.2 fix(deps): update module github.com/onsi/gomega to v1.36.2 fix(deps): update module github.com/opencontainers/runc to v1.2.4 fix(deps): update module github.com/shirou/gopsutil/v4 to v4.24.12 chore(deps): update dependency setuptools to ~=75.7.0 fix(deps): update module google.golang.org/protobuf to v1.36.2 fix(deps): update module github.com/vbauerster/mpb/v8 to v8.9.1 fix(deps): update module golang.org/x/net to v0.34.0 fix(deps): update module golang.org/x/tools to v0.29.0 chore(deps): update dependency setuptools to ~=75.8.0 fix(deps): update module google.golang.org/protobuf to v1.36.3 fix(deps): update module github.com/rootless-containers/rootlesskit/v2 to v2.3.2 fix(deps): update module github.com/containers/gvisor-tap-vsock to v0.8.2 chore(deps): update dependency pytest to v8.3.4 tomsweeneyredhat (2): [v5.4] Bump c/storage to v1.57.1, c/image v5.34.0, c/common v0.62.0 [v5.4] Bump Buildah to v1.39.0
Requires containers/storage#2155
RootFS.DiffIDs
, or refuse to pull it partially.RootFS.DiffIDs
to be present.RootFS.DiffIDs
exists and matches. This fixes the “view ambiguity” of partially-pulled layers.RootFS.DiffIDs
exists and we know the layer’s uncompressed digest, also require theRootFS.DiffID
value to match. This might be a compatibility break, but Docker requires these values anyway.DiffIDs
to empty values, if the layer does not have a TOC digest (so there is no risk of “view ambiguity”).See individual commit messages for details.