Giter Club home page Giter Club logo

Comments (19)

nalind avatar nalind commented on June 26, 2024 1

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).

from podman.

nalind avatar nalind commented on June 26, 2024 1

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.

from podman.

mtrmac avatar mtrmac commented on June 26, 2024 1

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.

from podman.

mtrmac avatar mtrmac commented on June 26, 2024 1

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.

from podman.

codingben avatar codingben commented on June 26, 2024

@nalind Can you please take a look? Thanks!

from podman.

mheon avatar mheon commented on June 26, 2024

@mtrmac PTAL

from podman.

codingben avatar codingben commented on June 26, 2024

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.

from podman.

mtrmac avatar mtrmac commented on June 26, 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?

from podman.

baude avatar baude commented on June 26, 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?

from podman.

nalind avatar nalind commented on June 26, 2024

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

from podman.

mtrmac avatar mtrmac commented on June 26, 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.

from podman.

codingben avatar codingben commented on June 26, 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.

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.

from podman.

codingben avatar codingben commented on June 26, 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

from podman.

nalind avatar nalind commented on June 26, 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.

from podman.

mtrmac avatar mtrmac commented on June 26, 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.

from podman.

nalind avatar nalind commented on June 26, 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.

from podman.

mtrmac avatar mtrmac commented on June 26, 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.)

from podman.

github-actions avatar github-actions commented on June 26, 2024

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

from podman.

mtrmac avatar mtrmac commented on June 26, 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.

from podman.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.