Podman manifest inspect does not display remote annotations

p5 commented 3 months ago

Issue Description

If you run podman manifest inspect quay.io/giuseppe/zstd-chunked:fedora-manifest on any remote manifest, you are unable to see the annotations which were added to the zstd:chunked manifest. Compare the output of the following commands:

$ skopeo inspect --raw docker://quay.io/giuseppe/zstd-chunked:fedora-manifest | jq .

$ podman manifest inspect quay.io/giuseppe/zstd-chunked:fedora-manifest

Steps to reproduce the issue

See above.

Push a manifest containing an image with annotations to any remote registry
Run podman manifest inspect on the local manifest (and see annotations)
Run podman manifest inspect on the remote manifest (and see no annotations)

Describe the results you received

As above

Describe the results you expected

I would expect podman manifest inspect to show all annotations on the manifests, since an annotation is a requirement for zstd images. When running skopeo inspect --raw, we can see the annotations, so I expected Podman to return the same.

podman info output

host:
  arch: amd64
  buildahVersion: 1.36.0
  cgroupControllers:
  - cpu
  - io
  - memory
  - pids
  cgroupManager: systemd
  cgroupVersion: v2
  conmon:
    package: conmon-2.1.10-1.fc40.x86_64
    path: /usr/bin/conmon
    version: 'conmon version 2.1.10, commit: '
  cpuUtilization:
    idlePercent: 96.86
    systemPercent: 1.56
    userPercent: 1.58
  cpus: 24
  databaseBackend: sqlite
  distribution:
    distribution: fedora
    variant: silverblue
    version: "40"
  eventLogger: journald
  freeLocks: 2045
  hostname: fedora
  idMappings:
    gidmap:
    - container_id: 0
      host_id: 1000
      size: 1
    - container_id: 1
      host_id: 524288
      size: 65536
    uidmap:
    - container_id: 0
      host_id: 1000
      size: 1
    - container_id: 1
      host_id: 524288
      size: 65536
  kernel: 6.8.11-300.fc40.x86_64
  linkmode: dynamic
  logDriver: journald
  memFree: 13290414080
  memTotal: 33364271104
  networkBackend: netavark
  networkBackendInfo:
    backend: netavark
    dns:
      package: aardvark-dns-1.11.0-1.fc40.x86_64
      path: /usr/libexec/podman/aardvark-dns
      version: aardvark-dns 1.11.0
    package: netavark-1.11.0-1.fc40.x86_64
    path: /usr/libexec/podman/netavark
    version: netavark 1.11.0
  ociRuntime:
    name: crun
    package: crun-1.15-1.fc40.x86_64
    path: /usr/bin/crun
    version: |-
      crun version 1.15
      commit: e6eacaf4034e84185fd8780ac9262bbf57082278
      rundir: /run/user/1000/crun
      spec: 1.0.0
      +SYSTEMD +SELINUX +APPARMOR +CAP +SECCOMP +EBPF +CRIU +LIBKRUN +WASM:wasmedge +YAJL
  os: linux
  pasta:
    executable: /usr/bin/pasta
    package: passt-0^20240624.g1ee2eca-1.fc40.x86_64
    version: |
      pasta 0^20240624.g1ee2eca-1.fc40.x86_64
      Copyright Red Hat
      GNU General Public License, version 2 or later
        <https://www.gnu.org/licenses/old-licenses/gpl-2.0.html>
      This is free software: you are free to change and redistribute it.
      There is NO WARRANTY, to the extent permitted by law.
  remoteSocket:
    exists: false
    path: /run/user/1000/podman/podman.sock
  rootlessNetworkCmd: pasta
  security:
    apparmorEnabled: false
    capabilities: CAP_CHOWN,CAP_DAC_OVERRIDE,CAP_FOWNER,CAP_FSETID,CAP_KILL,CAP_NET_BIND_SERVICE,CAP_SETFCAP,CAP_SETGID,CAP_SETPCAP,CAP_SETUID,CAP_SYS_CHROOT
    rootless: true
    seccompEnabled: true
    seccompProfilePath: /usr/share/containers/seccomp.json
    selinuxEnabled: true
  serviceIsRemote: false
  slirp4netns:
    executable: /usr/bin/slirp4netns
    package: slirp4netns-1.2.2-2.fc40.x86_64
    version: |-
      slirp4netns version 1.2.2
      commit: 0ee2d87523e906518d34a6b423271e4826f71faf
      libslirp: 4.7.0
      SLIRP_CONFIG_VERSION_MAX: 4
      libseccomp: 2.5.5
  swapFree: 8589930496
  swapTotal: 8589930496
  uptime: 2h 16m 4.00s (Approximately 0.08 days)
  variant: ""
plugins:
  authorization: null
  log:
  - k8s-file
  - none
  - passthrough
  - journald
  network:
  - bridge
  - macvlan
  - ipvlan
  volume:
  - local
registries:
  ghcr.io/rsturla:
    Blocked: false
    Insecure: false
    Location: ghcr.io/rsturla
    MirrorByDigestOnly: false
    Mirrors:
    - Insecure: true
      Location: localhost:5000/rsturla
      PullFromMirror: ""
    Prefix: ghcr.io/rsturla
    PullFromMirror: ""
  localhost:5000:
    Blocked: false
    Insecure: true
    Location: localhost:5000
    MirrorByDigestOnly: false
    Mirrors: null
    Prefix: localhost:5000
    PullFromMirror: ""
  search:
  - registry.fedoraproject.org
  - registry.access.redhat.com
  - docker.io
store:
  configFile: /var/home/admin/.config/containers/storage.conf
  containerStore:
    number: 1
    paused: 0
    running: 0
    stopped: 1
  graphDriverName: overlay
  graphOptions: {}
  graphRoot: /var/home/admin/.local/share/containers/storage
  graphRootAllocated: 1998678130688
  graphRootUsed: 285504368640
  graphStatus:
    Backing Filesystem: btrfs
    Native Overlay Diff: "true"
    Supports d_type: "true"
    Supports shifting: "false"
    Supports volatile: "true"
    Using metacopy: "false"
  imageCopyTmpDir: /var/tmp
  imageStore:
    number: 9
  runRoot: /run/user/1000/containers
  transientStore: false
  volumePath: /var/home/admin/.local/share/containers/storage/volumes
version:
  APIVersion: 5.1.1
  Built: 1717459200
  BuiltTime: Tue Jun  4 01:00:00 2024
  GitCommit: ""
  GoVersion: go1.22.3
  Os: linux
  OsArch: linux/amd64
  Version: 5.1.1

Podman in a container

No

Privileged Or Rootless

None

Upstream Latest Release

Yes

Additional environment details

N/A

Additional information

N/A

vrothberg commented 3 months ago

@giuseppe @mtrmac PTAL

That seems likely a time costly issue for users building multi-compression manifests.

Cc: @rhatdan

giuseppe commented 3 months ago

That seems likely a time costly issue for users building multi-compression manifests.

isn't it just a visualization issue or am I missing something?

@mtrmac is there any reason why we are hardcoding manifest.DockerV2ListMediaType for manifest inspect?

Dropping the hardcode value is enough to print the annotations.

diff --git a/pkg/domain/infra/abi/manifest.go b/pkg/domain/infra/abi/manifest.go
index 144740ec8..82d300192 100644
--- a/pkg/domain/infra/abi/manifest.go
+++ b/pkg/domain/infra/abi/manifest.go
@@ -189,7 +189,7 @@ func (ir *ImageEngine) remoteManifestInspect(ctx context.Context, name string, o
                if err != nil {
                        return nil, fmt.Errorf("parsing manifest blob %q as a %q: %w", string(result), manType, err)
                }
-               list, err := listBlob.ConvertToMIMEType(manifest.DockerV2ListMediaType)
+               list, err := listBlob.ConvertToMIMEType(manType)
                if err != nil {
                        return nil, err
                }

mtrmac commented 3 months ago

(I didn’t know Podman has this feature…)

History points at https://github.com/containers/podman/pull/7735#discussion_r496156316 for motivation; the https://github.com/containers/podman/commit/7ac8000cc1 commit, also, points out the existence of https://github.com/containers/podman/blob/3b07ae4557dcffc75fc4168c5355ace06bd0e822/pkg/api/handlers/libpod/manifests.go#L200 originally relying on the manifest inspect using a schema2 format … now using a misleading variable name.

AFAICT the local Podman variant doesn’t care / doesn’t say one way or the other about the output, the data is just a blob that is printed to stdout, and the man page doesn’t document anything about it.

In the remote Podman case, the data is clearly expected to be libimage/define.ManifestListData — both on the client and the server side, although confusingly using schema2 variable names in places

The local-storage code path in abi also uses a libimage/define.ManifestListData, confusingly using schema2 variable names.

So, assuming the “remote-registry inspect” code path is the rarely used one, that one should also return libimage/define.ManifestListData. That’s not currently available in the libimage API, but seems doable with a bit of refactoring.

I also think that ImageEngine.ManifestInspect should be returning define.ManifestListData instead of a raw (and completely undocumented!) []byte).

The remote-registry inspect code also seems to be returning data from a non-manifest-list v2s2 images ( https://github.com/containers/podman/pull/8100 ). With a warning, but, still. I don’t know how, if at all, that could fit into the better-typed API. Also note that this is unable to handle non-index OCI images.

My impression is that adding this feature has just been a mistake (note that the remote Podman client can’t unmarshal the returned data), but it might also be a mistake we can’t undo any more. I don’t know.

nalind commented 3 months ago

History points at #7735 (comment) for motivation; the 7ac8000cc1 commit, also, points out the existence of

https://github.com/containers/podman/blob/3b07ae4557dcffc75fc4168c5355ace06bd0e822/pkg/api/handlers/libpod/manifests.go#L200 originally relying on the manifest inspect using a schema2 format … now using a misleading variable name.

The define.ManifestListData attempts to be a union of all of the information that could be expressed in either a Schema 2 or OCI manifest, with the hope that all of its contents, whichever format it's in, will all be preserved across a JSON encoding/decoding cycle. Looking at it again, its Platform field, at least, doesn't manage to do that, so it isn't there yet.

The remote-registry inspect code also seems to be returning data from a non-manifest-list v2s2 images ( #8100 ). With a warning, but, still. I don’t know how, if at all, that could fit into the better-typed API. Also note that this is unable to handle non-index OCI images.

I think the swagger for the remote API only notes that this returns JSON, but beyond that I don't think it specifies anything about the format that it returns, so the remote path could switch to passing back whatever it received from the registry without even looking at it. If we extend define.ManifestListData to also include the fields which a non-index contains (Config and Layers), that information could even survive being decoded and then re-encoded for display. Granted, none of that would be especially pretty.

My impression is that adding this feature has just been a mistake (note that the remote Podman client can’t unmarshal the returned data), but it might also be a mistake we can’t undo any more. I don’t know.

I will note that I do not enjoy the ambiguity of this "sometimes this looks at a local image, sometimes it doesn't" behavior.

mtrmac commented 3 months ago

I’m sorry, in “My impression is that adding this feature has just been a mistake” I was referring to podman manifest inspect returning data about non-multi-platform v2s2 images; I don’t know enough to have any opinion on the “remote inspect” feature in general.

mtrmac commented 3 months ago

I think the swagger for the remote API only notes that this returns JSON, but beyond that I don't think it specifies anything about the format that it returns, so the remote path could switch to passing back whatever it received from the registry without even looking at it.

(The remote client is using the inspect endpoint (via InspectListData) to implement ManifestListClear. That should ideally detect/reject the non-list case. Possible, “not especially pretty”.)

github-actions[bot] commented 2 months ago

A friendly reminder that this issue had no activity for 30 days.

rhatdan commented 2 months ago

At the very least the podman-remote manifest inspect and podman manifest inspect should return the same data.

containers / podman