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

podman manifest inspect showing unexpected values for mixed-schema image structures #22196

Open
codingben opened this issue Mar 28, 2024 · 19 comments
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@codingben
Copy link

codingben commented Mar 28, 2024

Issue Description

We have pushed OCI-based image index and manifests. Podman returns wrong results about media type of image index.

Steps to reproduce the issue

Build OCI image index that will contain OCI image manifests, and push it to any container registry.

╰─$ podman pull quay.io/boukhano/cirros:6.1            
Trying to pull quay.io/boukhano/cirros:6.1...
Error: copying system image from manifest list: parsing image configuration: invalid mixed OCI image with Docker v2s2 config ("application/vnd.docker.container.image.v1+json")

Describe the results you received

╰─$ podman manifest inspect quay.io/boukhano/cirros:6.1                   
{
    "schemaVersion": 2,
    "mediaType": "application/vnd.docker.distribution.manifest.list.v2+json",
    "manifests": [
        {
            "mediaType": "application/vnd.oci.image.manifest.v1+json",
            "size": 418,
            "digest": "sha256:3a37dc3fd4e8460225ef5040308838ae344327c4901cffa653f3e157aae6c4f7",
            "platform": {
                "architecture": "amd64",
                "os": "linux"
            }
        },
        {
            "mediaType": "application/vnd.oci.image.manifest.v1+json",
            "size": 418,
            "digest": "sha256:cfefc66dad4addd779ff91774c1592f7a8cc87d3a905ea86bbcd9441045fe143",
            "platform": {
                "architecture": "arm64",
                "os": "linux"
            }
        }
    ]
}

Describe the results you expected

╰─$ docker manifest inspect quay.io/boukhano/cirros:6.1                                         1 ↵
{
   "schemaVersion": 2,
   "mediaType": "application/vnd.oci.image.index.v1+json",
   "manifests": [
      {
         "mediaType": "application/vnd.oci.image.manifest.v1+json",
         "size": 418,
         "digest": "sha256:3a37dc3fd4e8460225ef5040308838ae344327c4901cffa653f3e157aae6c4f7",
         "platform": {
            "architecture": "amd64",
            "os": "linux"
         }
      },
      {
         "mediaType": "application/vnd.oci.image.manifest.v1+json",
         "size": 418,
         "digest": "sha256:cfefc66dad4addd779ff91774c1592f7a8cc87d3a905ea86bbcd9441045fe143",
         "platform": {
            "architecture": "arm64",
            "os": "linux"
         }
      }
   ]
}

podman info output

If you are unable to run podman info for any reason, please provide the podman version, operating system and its version and the architecture you are running.

Podman in a container

No

Privileged Or Rootless

None

Upstream Latest Release

Yes

Additional environment details

N/A

Additional information

We have pushed OCI image index and manifests:

╰─$ curl -H "Accept: application/vnd.oci.image.index.v1+json" "https://quay.io/v2/boukhano/cirros/manifests/6.1"   

{"schemaVersion":2,"mediaType":"application/vnd.oci.image.index.v1+json","manifests":[{"mediaType":"application/vnd.oci.image.manifest.v1+json","size":418,"digest":"sha256:3a37dc3fd4e8460225ef5040308838ae344327c4901cffa653f3e157aae6c4f7","platform":{"architecture":"amd64","os":"linux"}},{"mediaType":"application/vnd.oci.image.manifest.v1+json","size":418,"digest":"sha256:cfefc66dad4addd779ff91774c1592f7a8cc87d3a905ea86bbcd9441045fe143","platform":{"architecture":"arm64","os":"linux"}}]}
@codingben
Copy link
Author

@nalind Can you please take a look? Thanks!

@mheon
Copy link
Member

mheon commented Mar 28, 2024

@mtrmac PTAL

@codingben
Copy link
Author

Thanks for looking on it. @nalind debugged and saw that layer/config still using Docker and not OCI, and that's why Podman doesn't work:

╰─$ skopeo inspect --raw docker://quay.io/boukhano/cirros@sha256:e96e9fe082ec1c9c056144f58b52de10314adb79353f35298caa2cd25c79d83c | jq
{
  "schemaVersion": 2,
  "mediaType": "application/vnd.oci.image.manifest.v1+json",
  "config": {
    "mediaType": "application/vnd.docker.container.image.v1+json",
    "size": 329,
    "digest": "sha256:7760637f74788fa570057d4c2e7b5c21d996863744d6049807c218f01154fa74"
  },
  "layers": [
    {
      "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip",
      "size": 20787697,
      "digest": "sha256:97565d0bbd9530b3349cabff269322e8686a4f3add243e72814ba51630e49be9"
    }
  ]
}

While image index and image manifests are based on OCI schemas.

@nalind
Copy link
Member

nalind commented Mar 28, 2024

Out of band, we have an image with an OCI media type, but which is using Docker media types to describe its config blob and layers:

{
  "schemaVersion": 2,
  "mediaType": "application/vnd.oci.image.manifest.v1+json",
  "config": {
    "mediaType": "application/vnd.docker.container.image.v1+json",
    "size": 329,
    "digest": "sha256:7760637f74788fa570057d4c2e7b5c21d996863744d6049807c218f01154fa74"
  },
  "layers": [
    {
      "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip",
      "size": 20787697,
      "digest": "sha256:97565d0bbd9530b3349cabff269322e8686a4f3add243e72814ba51630e49be9"
    }
  ]
}

being generated by https://github.com/kubevirt/containerdisks/blob/main/pkg/build/build.go with changes from https://github.com/kubevirt/containerdisks/compare/40fc44a45797c5e02bfdf72d1f161fc0bb155254..c39ddf6c0af1b76817541c0cdae8e744003b252d applied.

It looks like passing tarball.WithMediaType() as the second argument to the tarball.LayerFromOpener() call would change the layer's media type and running img through mutate.ConfigMediaType() would change its config blob's type.

But that leaves the question of whether or not we should be balking at such an image anyway. The spec seems to advise that images that are concerned with portability should be using OCI types for these things, and that seems easy enough to fix here, but the spec notes that implementations mustn't error out when encountering unknown types, and it looks to me like we're doing that here (for certain values of "unknown", anyway).

@nalind
Copy link
Member

nalind commented Mar 28, 2024

being generated by https://github.com/kubevirt/containerdisks/blob/main/pkg/build/build.go with changes from https://github.com/kubevirt/containerdisks/compare/40fc44a45797c5e02bfdf72d1f161fc0bb155254..c39ddf6c0af1b76817541c0cdae8e744003b252d applied.

... or reverted, as it may be.

@mtrmac
Copy link
Collaborator

mtrmac commented Mar 28, 2024

But that leaves the question of whether or not we should be balking at such an image anyway. The spec seems to advise that images that are concerned with portability should be using OCI types for these things, and that seems easy enough to fix here, but the spec notes that implementations mustn't error out when encountering unknown types, and it looks to me like we're doing that here (for certain values of "unknown", anyway).

The spec says

Implementations storing or copying image manifests MUST NOT error on encountering a value that is unknown to the implementation.

That applies to skopeo copy, but not really to pulls which must consume the data.

It would be ~easy enough to handle some of the code paths; e.g. Inspect can ignore the manifest media type and choose an implementation based on just the config’s media type.

But I don’t think it’s longer-term reasonable, because the matrix of combinations blows up and capabilities can no longer be relied on.

E.g., (until recent Docker’s vendor-specific extension), “OCI images don’t support health checks” was true. If we support “OCI images with v2s2 configs”… what does that mean?

If we support “OCI images with v2s2 configs”, do we need to implement a “convert an OCI image to really-follows-spec-OCI-image” operation?

Hypothetically, if OCI config annotations are defined: What does it mean to have a v2s2 config with a an OCI-config-specific annotation?

@baude
Copy link
Member

baude commented Mar 28, 2024

@codingben @nalind @mtrmac thanks for all the write ups ... i wanted to make sure we captured this for the future. it also seems to me that the title of the issue no longer reflects what is going on ... should it be updated for clarity?

@mtrmac
Copy link
Collaborator

mtrmac commented Mar 28, 2024

All that said, this output

$ podman manifest inspect quay.io/boukhano/cirros:6.1                   
{
    "schemaVersion": 2,
    "mediaType": "application/vnd.docker.distribution.manifest.list.v2+json",

is either a bug, or maybe an intentionally-permissive implementation somehow trying to fall back to v2s2 based on the inconsistent per-platform instances.

@nalind
Copy link
Member

nalind commented Mar 28, 2024

I expect that was built using kubevirt/containerdisks#81, where the mediaType values being used are all hard-coded.

@mtrmac
Copy link
Collaborator

mtrmac commented Mar 28, 2024

Was the image under that tag overwritten / replaced? At least right now, skopeo inspect --raw shows the image to use an OCI index media type.

@codingben
Copy link
Author

Was the image under that tag overwritten / replaced? At least right now, skopeo inspect --raw shows the image to use an OCI index media type.

It should be OCI index media type and OCI image manifests, I just updated it again:

╰─$ docker manifest inspect quay.io/boukhano/cirros:6.1     
{
   "schemaVersion": 2,
   "mediaType": "application/vnd.oci.image.index.v1+json",
   "manifests": [
      {
         "mediaType": "application/vnd.oci.image.manifest.v1+json",
         "size": 418,
         "digest": "sha256:a0e5b0df033c10034371d9711d90771816f349b57e246cf95b17b52b086a0d26",
         "platform": {
            "architecture": "amd64",
            "os": "linux"
         }
      },
      {
         "mediaType": "application/vnd.oci.image.manifest.v1+json",
         "size": 418,
         "digest": "sha256:7aa5762d40d1bbc545210629f9ff5a2036179d63551a5abbfe13f614f6cb4962",
         "platform": {
            "architecture": "arm64",
            "os": "linux"
         }
      }
   ]
}

If you run podman manifest inspect quay.io/boukhano/cirros:6.1, it'll return Docker manifest list instead of OCI image index now.

@codingben
Copy link
Author

codingben commented Mar 28, 2024

@mtrmac Because of it, we decided that in our production registry quay.io/organization/containerdisks - We'll use Docker schemas and not OCI because of this issue [1].

So I'll leave quay.io/boukhano/cirros:6.1 to use OCI image index and OCI image manifests for testing purposes of this issue.

[1] https://github.com/kubevirt/containerdisks/compare/40fc44a45797c5e02bfdf72d1f161fc0bb155254..c39ddf6c0af1b76817541c0cdae8e744003b252d

@mtrmac mtrmac changed the title Podman pulls Docker manifests list and not OCI image index podman manifest inspect showing unexpected values for mixed-schema image structures Mar 28, 2024
@mtrmac
Copy link
Collaborator

mtrmac commented Mar 28, 2024

I have gone ahead and retitled this to only describe the podman manifest inspect behavior, under the assumption that we don’t want to commit to fully supporting mixed-schema images. (Fixing this might mean just refusing the operation, or some earlier operation, not necessarily adding support.)

I’m open to the argument that this should actually be supported, and that this issue should track that instead.

@nalind
Copy link
Member

nalind commented Mar 28, 2024

It would be ~easy enough to handle some of the code paths; e.g. Inspect can ignore the manifest media type and choose an implementation based on just the config’s media type.

But I don’t think it’s longer-term reasonable, because the matrix of combinations blows up and capabilities can no longer be relied on.

I'm not advocating that we generate such images.

E.g., (until recent Docker’s vendor-specific extension), “OCI images don’t support health checks” was true. If we support “OCI images with v2s2 configs”… what does that mean?

If podman can parse the configuration blob (in whatever format that it's in), and that format provides settings that podman understands, it should pay attention to them. It already has to vary how it parses manifests and config blobs in order to avoid discarding information that's specific to one format or another. The main question for me is whether it does so by directly consulting the mediaType in the config descriptor, or if it just assumes based on the mediaType of the image manifest as a whole.

If we support “OCI images with v2s2 configs”, do we need to implement a “convert an OCI image to really-follows-spec-OCI-image” operation?

It's been a while since I've dug down into the image library's copy logic, but I thought that it was already prepared to convert data from one format to another, and to do so at multiple levels, when the destination doesn't accept an image in its as-is form. Granted, I don't remember it trying to convert an image into "the format the manifest says it's in, with anything that might be considered inconsistent ironed out all the way down", because it's not something I thought it might need to try.

github.com/moby/docker-image-spec/specs-go/v1.DockerOCIImageMediaType has the same value as github.com/containers/image/v5/manifest.DockerV2Schema2ConfigMediaType, and I think that means we're probably going to see OCI images with that type in their config descriptors at some point.

Oh no, maybe I am advocating that we start generating such images.

Hypothetically, if OCI config annotations are defined: What does it mean to have a v2s2 config with a an OCI-config-specific annotation?

Annotations in the config descriptor that are applicable only to config data of a different mediaType than the one in the descriptor? I might want the implementation to feign ignorance of that annotation in that case.

@mtrmac
Copy link
Collaborator

mtrmac commented Mar 28, 2024

github.com/moby/docker-image-spec/specs-go/v1.DockerOCIImageMediaType has the same value as github.com/containers/image/v5/manifest.DockerV2Schema2ConfigMediaType, and I think that means we're probably going to see OCI images with that type in their config descriptors at some point.

Great catch, I didn’t notice that constant.

But that constant is actually not used anywhere in the docker/docker or docker/cli codebase - they use the OCI config media type for configs generated using docker-image-spec.

Oh no, maybe I am advocating that we start generating such images.

I do agree that Docker compatibility, or similar concerns, might lead us to support some JSON fields, and some MIME types, not in the official OCI specification.

I’d prefer not to generalize that to the ambition to support any possible combination of MIME types that shows up once. Just saying “no, that’s a bug, fix it before your first release” is, I think, frequently the better option for the ecosystem as a whole. It’s probably a fairly small one-time fix in one codebase, vs. adding support for that quirk not just in c/image, but also in any other possible consumer over time, and maintaining that part of the test matrix for a long time.

@nalind
Copy link
Member

nalind commented Mar 28, 2024

github.com/moby/docker-image-spec/specs-go/v1.DockerOCIImageMediaType has the same value as github.com/containers/image/v5/manifest.DockerV2Schema2ConfigMediaType, and I think that means we're probably going to see OCI images with that type in their config descriptors at some point.

Great catch, I didn’t notice that constant.

But that constant is actually not used anywhere in the docker/docker or docker/cli codebase - they use the OCI config media type for configs generated using docker-image-spec.

I hadn't checked on that, thanks for doing the legwork. Something to keep an eye out for.

Oh no, maybe I am advocating that we start generating such images.

I do agree that Docker compatibility, or similar concerns, might lead us to support some JSON fields, and some MIME types, not in the official OCI specification.

I’d prefer not to generalize that to the ambition to support any possible combination of MIME types that shows up once. Just saying “no, that’s a bug, fix it before your first release” is, I think, frequently the better option for the ecosystem as a whole. It’s probably a fairly small one-time fix in one codebase, vs. adding support for that quirk not just in c/image, but also in any other possible consumer over time, and maintaining that part of the test matrix for a long time.

That's entirely reasonable, since I'd now characterize the origin of this issue as an attempt to change the mediaType values in images that the containerdisks package produces, an attempt that didn't quite catch everything that needed to be changed.

The possible future compatibility concern is my main concern as well. Is there anything that you think we can or should do in the near-term to be better prepared for a case where this happens? I can easily imagine wanting to be able to provide a healthcheck configuration in an OCI image and trying to swap in a different config blob type to make it work. I haven't checked if the various registry implementations would accept such a thing, though, so that may not be an option yet.

@mtrmac
Copy link
Collaborator

mtrmac commented Mar 28, 2024

As a vague guess based on past experience, I agree I’d expect strict Quay validation to be one of the main constraints.

The thing is, “being prepared” can be argued both ways:

  • completely ignore MIME types and just parse the fields that exist, and (due to Go’s questionable-but-pragmatically-convenient default behavior of silently ignoring unknown JSON fields) that will probably work when encountering future formats — work somehow
  • or, strictly enforce MIME format match (and reject unknown fields?), so that unknown critical fields are not ignored, and users are told that they must upgrade.

I think, on the net, for forward compatibility, a combination of strictly enforcing MIME formats, and silently ignoring unknown fields, is a good option, because it gives future formats all the tools necessary: they can represent non-critical features as new fields in existing MIME types (and they will be ignored by old consumers), and critical features as new MIME types (and they will cause old consumers to fail).

(Note that skopeo copy will copy known image MIME types, unknown artifacts, and unknown image MIME types, all the same way; using an unknown image config “only” causes the layer compression/decompression code to disengage. So, in that sense, we are forward-compatible; but podman pull / podman run does refuse to work with unexpected MIME types. And, now that non-image artifacts are becoming increasingly prevalent, I think it rather has to continue refusing that.)

But then, none of the above is an argument one way or the other for the v2s2-config-in-OCI-manifest situation. That’s not really a “forward compatibility” case, and we know it’s not an artifact.


(WRT the specific health check example, as it happens, the c/image abstraction of Inspect, for probably path dependency reasons without a deeper cause, doesn’t contain the health check field at all, and it is handled in Buildah/Podman/common.

If we want to add support Docker’s healthcheck-in-OCI extension (and we probably do, eventually), c/image will have to learn about the field, so that c/image’s conversions between v2s2 and OCI don’t discard it. And that would be a natural time to add the field to types.Image.Inspect; then the Podman etc. consumers can migrate to that future c/image abstraction, instead of adding another handler for OCI specifically.)

Copy link

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

@mtrmac
Copy link
Collaborator

mtrmac commented Apr 29, 2024

Looking at the manifest inspect behavior, the explanation is:

I think the underlying issue here is that podman manifest … is designed to create multi-arch images; there is no podman manifest pull.

So, combining podman pull + podman manifest inspect does not fail but it seems to not have been a part of the initially-contemplated design, at least to the extent that inspect command intentionally optimizes for the case of pushing newly-created images, at the cost of fidelity when showing data about existing ones.

And, like in Buildah, changing the “WIP objects exist in both formats simultaneously” assumption would be a fairly invasive change.

Meanwhile, skopeo inspect --raw is available to inspect external on-registry objects, so podman manifest inspect does not need to prioritize the use case.

So, overall, leaving things as they are seems broadly reasonable to me, except perhaps that podman-manifest-inspect.1 should document what the command is doing in a bit more detail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

No branches or pull requests

5 participants