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

Remove artifact manifest #999

Merged
merged 1 commit into from
Apr 13, 2023
Merged

Conversation

jonjohnsonjr
Copy link
Contributor

@jonjohnsonjr jonjohnsonjr commented Jan 24, 2023

The artifact manifest does not confer any additional benefits beyond the existing image manifest. Drop it for the sake of backward compatibility.

Signed-off-by: Jon Johnson [email protected]

Edit: Please read the specification before brigading this Pull Request, thanks!

@vbatts
Copy link
Member

vbatts commented Jan 25, 2023

well, the removed references are not in any tagged releases (non-RC), but I recall folks using this. It would be good to hear some feedback on this before just axing it.

@imjasonh
Copy link
Member

For what it's worth, I support this change. There are two headline changes in v1.1: the references API, and the artifact manifest. Of the two, references has gotten by far more attention and iteration, and it shows. It's not perfect, but it's something I think folks can use and gain clear value from. Artifact manifests, less so.

The user need for an artifact manifest is also quite a bit lower. Inasmuch as any end user knows anything about the low-level APIs we're talking about, they can at least understand the value of being able to attach things to things and list them back, without polluting the tag list. Will any user get any real value from a signature or SBOM being stored as an image-typed manifest vs an artifact-typed manifest? I doubt it.

As much as I hate to admit it, I think there's sufficient confusion in the ecosystem already about what an "OCI Artifact" even is, that we should step back and consider whether we want to muddy that even more. To everyone except a handful of people, an "OCI Artifact" is an image-typed manifest with a non-standard config.mediaType. That's what opencontainer/artifacts has documented publicly for three and a half years. That's the usage that Docker Hub advertises. I'm afraid adding a new "artifact" manifest type -- that nobody but deeply involved maintainers and implementers will ever know to be different than "OCI Artifacts" as it's commonly used -- will be too confusing. "Why doesn't Docker Hub accept my artifact? They said they did!" -- "Well, y'see, because 'OCI Artifact' actually means..." -- it's not a great story for anybody.

Does it bother me that we could end up storing SBOMs and signatures and cat pictures and everything else in image-typed manifests? It sure does, as I imagine it does for all of us. But that misnomer is small in comparison to the confusion that would be caused by having two different "Artifacts" in the OCI lexicon.

Finally, as far as I know, there are no clients begging us to support artifact-typed manifests immediately. Any that do would only work with artifact-typed-manifest-supporting registries, of which there are none. Those manifests would not be portable to any other registries immediately, and would not be portable to all registries, ever. It's a recipe for headaches, and it's still completely avoidable.

So I think we should back out the artifact manifest before 1.1, add a doc describing that "OCI Artifacts" are image-typed manifests with a non-config config.mediaType (replacing content in the artifacts repo), and consider if there are reasons to try to support a separate artifact-typed manifest in a possible future v1.2.

This lightens the v1.1 release, making it easier to adopt for clients and registries, easier to write conformance for, easier to attain conformance for, and generally makes all our lives easier, as well as users'.

imjasonh
imjasonh previously approved these changes Jan 25, 2023
@toddysm
Copy link

toddysm commented Jan 25, 2023

I agree with @vbatts - shouldn't we have a discussion before we submit the PR?

I am trying to understand what is the goal of removing the artifactType and will appreciate if @jonjohnsonjr or @imjasonh can be more explicit about that. Stating that "we don't see value" is as helpful as me saying that "there is value" :) If it is a backwards compatibility, the goal was that we won't make the artifactType required for those purposes. I also understand that by hacking around the image-type manifest we can achieve a good set of known scenarios (not all though as the one I list below) but it is messy, limiting, and not forward looking.

Two very concrete points:

  • Docker Hub already supports artifactType. I have tested this with their APIs. It will be good to have somebody from Docker Hub to comment on their plans for the future.
  • About the ask for scenario. We are using the artifactType already for various scenarios but I see the point that those can be implemented with hacking around the image-type manifest (of course, I would prefer not to). Though, one scenario where I think this hacking will not be applicable is annotation artifacts where you want to only add annotation to whatever is stored in the registry. Either this will require every time to generate a bunch of empty blobs (which IMHO is really bad experience) or break the 1.0 registries by relaxing the need for blobs in the manifest, which defeats the purpose of this proposal.

@sudo-bmitch remember we had a discussion in #903 There is need to only update annotations if the support policy changes.

@imjasonh
Copy link
Member

My intention wasn't to merge this without discussion. Any change in this repo requires >half of approvers, which historically means any change takes weeks to merge anyway.

FWIW I prefer having these kinds of discussions attached to actual practical diffs, since otherwise folks can spend weeks agreeing in principle to a change, then spend weeks coming to agreement about specific changes. Better to collapse the two windows together I think.

Also, artifactType won't be going away with this change. ArtifactType is still the field in the descriptor that holds the hoisted config.mediaType in a referrers API response (incl tag fallback).

This topic is on the agenda for tomorrow's call if folks want to come and discuss, or feel free to comment and discuss here.

@toddysm
Copy link

toddysm commented Jan 25, 2023

GitHub says the merging can be performed automatically with 2 approving reviews - please excuse my ignorance about the process :)

Wouldn't it be even more confusing if we had artifactType but no artifacts manifest?

Copy link
Contributor

@sudo-bmitch sudo-bmitch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewing to hold this open for more discussion.

I lean against completely removing the artifact manifest. But I do agree that it has very little added value and creates portability issues if used prematurely, so it's worth considering.

@imjasonh
Copy link
Member

GitHub says the merging can be performed automatically with 2 approving reviews - please excuse my ignorance about the process :)

I stand corrected.

Wouldn't it be even more confusing if we had artifactType but no artifacts manifest?

Not if "OCI Artifacts" uses the definition that's been in common usage for years. I do think there's some confusion this way, but much less than having two "Artifacts".

Copy link
Member

@sajayantony sajayantony left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding this manifest type is non-breaking and warrants conscious adoption by clients since producers of this type would risk their content not being portable across registries that don’t implement 1.1.

The value adds of this, which were discussed during the working group, was to attach non-images type of content to images due to specific limitations of the image manifest.

  • Image manifest requires a blob

One use case was represented by @toddysm during this call. But the larger point IMHO was that it was discussed in the working group at length and the need summarized with many participants. Capturing that context back here is non-trivial. Axing this would be a cost for existing implementations like Docker/zot/acr/regctl/oras etc. Yes, this is the risk of picking up an RC but none the less, it is destabilizing to partners at a point when we are discussing a release of v1.1.

There were a set of valid points brought up during the call by @AaronFriel and I hope they capture in following comments or in the notes to describe how Pulumi was thinking about leveraging it.

@lachie83
Copy link
Contributor

Hey folks, sorry I couldn't make the weekly call earlier this morning as I understand this was discussed, I will watch the recording once posted.

I would really like to make sure we are adhering to the process set forth in the WG docs. My main concern is that there was a proposal agreed upon by the participants of the Reference Types WG which was subsequently reviewed and merged by the maintainers. Following merge, the process to cut the 1.1 distribution and image spec release commenced and has been going on for the last couple of months. To drop a PR which basically reverts months of collaboration and agreement from a working group on a Tuesday afternoon without any prior discussion with the participants of that working group, undermines the very purpose of a working group. As such, I don't believe that this PR is in the spirit of collaboration that working groups set out to facilitate. It is my hope that we can identify and raise concerns like this as part of the working group process rather than having late binding ad-hoc PRs.

@jonjohnsonjr
Copy link
Contributor Author

To drop a PR which basically reverts months of collaboration and agreement from a working group on a Tuesday afternoon without any prior discussion with the participants of that working group, undermines the very purpose of a working group. As such, I don't believe that this PR is in the spirit of collaboration that working groups set out to facilitate. It is my hope that we can identify and raise concerns like this as part of the working group process rather than having late binding ad-hoc PRs.

I disagree wholeheartedly. The PR proposes a concrete change based on discussions from the last several weeks of OCI dev calls.

@TomasB12345
Copy link

TomasB12345 commented Jan 26, 2023 via email

@AaronFriel
Copy link
Contributor

I'm sure everyone realizes that, but it should be stated that as in software delivery, it's always risky to make significant changes right before shipping. You can surprise your customers, and worse, sometimes you can surprise yourself. This is a significant decision to make, and if I have a vote (I don't think I do), I'd vote against.

On the technical merits the author is correct that clients MAY today conspire to deceive registries and upload and download arbitrary artifacts, including cat pictures, as layers. OCI 1.0 permitted that.

However, I believe there are three groups to consider: registries, clients, and the OCI committee. In considering each of these, I think there is reason to reject the proposed change.

Registry support for artifacts

Registries MAY reject cat gifs, and both clients and registries MAY fail in spectacular ways on encountering a gif instead of a tarball, expecting layer to imply image layer blobs. Support for arbitrary blobs and config media types is poor. A clean break to a new manifest type prompted clean implementations, and that makes for a more stable target for new implementers to adopt.

Ensuring that OCI 1.1 conformance truly does still imply Artifact support, except in name requires a re-review of the language in other specifications to ensure that the artifact ecosystem can rely on OCI 1.1 registries to support their use cases and that, e.g.: a conformant registry cannot block content because it is not a valid image layer, arbitrary config types must be accepted, and so on.

A majority should only approve this PR conditional on substantial investments in the other specifications, which will inform OCI 1.1 conformance tests, to ensure that any registry advertising and evaluating against it will support all of the use cases the broader ecosystem is exploring.

Client adoption of artifacts

There is immense industry interest in Artifact, and ensuring that registries must support Artifact to pass OCI 1.1 conformance tests ensures that, while some things (downgrade/upgrade behavior, for one) are underspecified, at least clients and registries can begin to interoperate with a firm understanding that they are handling items more generic than images.

This is the weakest argument, but I think there are large positive externalities of embracing the Artifact specification as it is, as acknowledging that OCI is not just for images. Clear communication to the broader world is necessary to explain this change, and OCI's intentionality around future support for artifacts.

As one of the only formal outputs of the group is specifications, not press releases, I believe a majority should only approve this PR if they believe it clearly communicates the future of artifact support in OCI registries.

Reputational risk to OCI

Two release candidates were made with Artifact included, a signal to the broader ecosystem of a shift in OCI's thought toward generic use, and a sign that they could begin working toward implementation. I believe most of the attendees of the OCI Dev call indicated they already support the RC spec (albeit in preview form) and a burgeoning client ecosystem is likewise already working to support Artifact, targeting OCI 1.1 compliant registries.

If accepted, implementers will, rationally, be less willing to be early adopters of future release candidates and may not be able to provide feedback to the WG. Removing OCI Artifact now will prompt skepticism, and that needs to be assuaged with clear communication, which I don't believe this pull request represents.

A majority should only approve this PR if they believe that it will not impact future specification work.

@sudo-bmitch
Copy link
Contributor

sudo-bmitch commented Jan 27, 2023

This and opencontainers/distribution-spec#365 have resulted in a lot of debate within the last few weeks of OCI meetings because there's an issue that OCI has never had to solve before. That issue is how do we add new manifests without breaking portability (see our charter -> values). In the past, this has not been an issue because producers were very conservative in adopting OCI media types over the Docker media types. A few notable examples of images still using the Docker media types for their manifest:

  • ghcr.io/opencontainers/distribution-spec/conformance:main
  • registry:2
  • alpine, busybox, debian, and any other official Docker image I've checked
  • gcr.io/distroless/base
  • bitnami/etcd:latest

Since image build tooling has avoided a rapid switch in the past, the user experience has been rather painless to adopt OCI media types today. Registries had plenty of time to upgrade, and the runtimes that don't support the OCI media types have mostly been upgraded by now.

In the working group, we were concerned about a rapid adoption of the new artifact manifest, and provided the following guidance for an upgrade path:

  • Registries without new API or media type support can use the image manifest plus digest tags to create artifacts that refer to other manifests.
  • As registries are upgraded, the new API can be used for more efficient queries and less tag clutter. The extended Image manifest would still be used for portability to older registries.
  • When enough registries support the new media types, a transition can happen to those media types for artifacts.

What we didn't specify is what "enough registries" looks like. I wanted to trust clients to follow historical behavior, but the last few meetings have proved me wrong since clients are already being designed to prefer or exclusively support the new manifest type, skipping the recommended second phase, which has triggered the current debate. We could defer this debate by accepting this PR but that doesn't solve the underlying issue.

From here, I think there are several options:

  1. Indefinitely postpone the artifact manifest. If we pull it out of the release, I'd want to create a feature branch in OCI to denote that it is being considered for a future date.
  2. Define a policy for upgrading that gives consumers time to upgrade without tooling forcing breaking changes on them.
  3. Adopt the spec as is and allow breaking changes to be forced on consumers by anyone that wants to use it today.

The risk of option 1 is we never find a way to add new functionality in OCI. If we go that route, I'd push to get the artifact manifest included in a feature branch separate from main to support external development. The risk of option 3 is that breaking changes will be pushed on users with little end-user value over the current image manifest solution, going against our charter and values.

My own preference is for option 2, where we give registries, self hosted environments, and tooling developers, time to upgrade. Then when the change is made by artifact producers, it's transparent to most end users, rather than a breaking change. This happens today in almost every software project, where end users are given time to upgrade before breaking their workflows.

I'd like to see us specify that producers of content MUST default to OCI v1.0 manifest types for $x time period after the release of OCI v1.1 to be considered OCI conformant. Non-conformant tooling can be delisted from the OCI pages, and we can use the trademark to require them to remove any OCI claims from their project. If we opt to go this direction, then we just need to agree on a reasonable value for $x.

@AaronFriel
Copy link
Contributor

Define a policy for upgrading that gives consumers time to upgrade without tooling forcing breaking changes on them.

I don't follow - is the concern that clients will be forced to handle Artifact media types? I think almost all tooling that currently interacts with registries is either image oriented and will continue to produce and consume images, or is artifact oriented and prefers this clean break.

The middle road: all artifacts appear as images, is likely to be the most confusing to legacy clients.

@sudo-bmitch
Copy link
Contributor

Define a policy for upgrading that gives consumers time to upgrade without tooling forcing breaking changes on them.

I don't follow - is the concern that clients will be forced to handle Artifact media types?

The purpose of the working group was to define how to attach additional data to an image. So there's an expectation that what we create should not break tooling that works with images. Tooling like pull through proxies, vulnerability scanners, signing, SBOM generators, image builders, and runtimes. Each of these may encounter an artifact for a variety of reasons. Perhaps someone wants to sign an artifact with their signing tool, or scan it for vulnerabilities before admitting it into their organization's network. They may be copying the artifact from a dev/CI registry to the production registry, and the production registry may only be upgraded after they have tested the upgrade in development. In each of these cases, forcing a new manifest type on end users, without adequate time for them to upgrade their servers and tooling, means it's a breaking change. That looks really bad on OCI as the organization creating standards for interoperability.

I think almost all tooling that currently interacts with registries is either image oriented and will continue to produce and consume images, or is artifact oriented and prefers this clean break.

I write tooling that works with both, as do several other people pushing for this PR, and I work with a lot of clients in brown fields that are trying to improve security in their software pipelines. I don't believe we're pushing for a "clean break", rather I'm looking for a controlled transition.

The middle road: all artifacts appear as images, is likely to be the most confusing to legacy clients.

Tooling already exists that ships artifacts using the image manifest. Everyone has had time to adopt it and deal with any issues resulting from it.

If anyone isn't concerned with interoperability, then they don't need OCI. They can build their own servers, their own clients, and their own APIs. The value we have with approving specs in OCI is that interoperability between multiple vendors and projects.

@AaronFriel
Copy link
Contributor

My question is - under what scenario do you expect artifacts-pretending-to-be-images to behave better than artifacts disclosing themselves as manifests?

Either will break tooling that inspects the content and expects an image with layers, and layers whose blobs are valid layer tarballs.

@vbatts
Copy link
Member

vbatts commented Jan 27, 2023

just uploaded yesterday's community call: https://youtu.be/w39590Jn5zg

@vbatts
Copy link
Member

vbatts commented Jan 27, 2023

How did this level of concern not come up during the entirety of the of the WG that covered this in depth?
This is fairly disruptive and feels contentious. 😕
(still watching the video...)

@imjasonh
Copy link
Member

imjasonh commented Jan 27, 2023

How did this level of concern not come up during the entirety of the of the WG that covered this in depth? This is fairly disruptive and feels contentious. 😕

First of all, I think some of it did come up, and we just powered through it. @jonjohnsonjr was regrettably not able to participate in the WG for a few months due to personal leave, which I think also contributed (absolutely not blaming, just noting).

Now as we're getting closer and have RCs of the spec, and folks are looking at how they would implement it, both from the registry side and as a client / producer, it's becoming apparent (to me) that this could cause a fracture in the ecosystem -- some clients produce/consume artifact manifests and are locked in to registries that support it, some clients produce/consume only image manifests and are more portable, and clients that want the best of both worlds have to have cumbersome logic to translate between the two.

Even worse for clients that try to paper over the different manfiests, the digests of ~equivalent manifests in image- and artifact-ese can't be associated with one another, so you'll proliferate manifests as you try to move manifests across registries with different support. This specific case is the one that did the most to convince me that artifact manifests would be more trouble than they're worth.

A theme in the call was that there's a tension between the idea that OCI should innovate and provide more value to users, and the idea that OCI should take care to ensure portability across registries. I think we spent much more time and energy in the WG making sure that referrers were innovative, useful and portable (with fallback tag), and much less time thinking about whether artifact manifests would be both (and it shows; they are not).

@@ -35,7 +35,6 @@ For the media type(s) that this document is compatible with, see the [matrix][ma
Implementations MUST support at least the following media types:

- [`application/vnd.oci.image.manifest.v1+json`](manifest.md)
- [`application/vnd.oci.artifact.manifest.v1+json`](artifact.md)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just move this line to SHOULD support, then no need to remove the entire artifact spec?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be a good change regardless. Basically, all registries MUST support image manifests, and SHOULD (or even MAY) support the artifact manifest.

Without this change, a registry could choose to only support the artifact manifest, which would probably only cause a deeper fracture in the ecosystem.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without any change, registries MUST support both the artifact and image manifests.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would lead to a great deal of confusion for OCI 1.1 compliance and support for artifacts. With so many registries already adopting Artifact in it's RC form, what is gained by weakening this MUST to SHOULD?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AaronFriel it lets registries adopt the references API (much more valuable imo) and claim 1.1 conformance. If they have to adopt both to be 1.1, they might prefer not to invest in either.

Copy link
Member

@imjasonh imjasonh Jan 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without any change, registries MUST support both the artifact and image manifests.

Oh you're correct, I misread.

I'd still like to advocate for "MUST support at least one of these media types"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AaronFriel it lets registries adopt the references API (much more valuable imo) and claim 1.1 conformance. If they have to adopt both to be 1.1, they might prefer not to invest in either.

What utility do references have absent artifacts? If registries can claim conformance with OCI 1.1 but no one can upload an sbom, NPM package, cat gif, etc., I don't see the point of OCI 1.1.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clients have been successfully uploading SBOMs, NPM packages, cat pictures, etc., for a couple years now at least under 1.0.

1.1 will codify some of those patterns and augment them with a first-class supported API that doesn't require polluting tag hacks.

@sudo-bmitch
Copy link
Contributor

My question is - under what scenario do you expect artifacts-pretending-to-be-images to behave better than artifacts disclosing themselves as manifests?

Either will break tooling that inspects the content and expects an image with layers, and layers whose blobs are valid layer tarballs.

  • A pull through proxy will pass through artifacts using the image manifest unchanged.
  • A signing tool can sign artifacts using the image manifest
  • An artifact shipped with an image manifest can be pushed to an OCI v1.0 registry and used by tooling aware of the artifacts

None of those scenarios would work with the artifact manifest unless the tooling is upgraded first. I think cosign, ORAS, and Helm are also evidence that artifacts pushed using the image manifest have not had a significant negative impact to existing tooling.

@afflom
Copy link

afflom commented Jan 27, 2023

At the very least, Can OCI publish something affirming its stance on the OCI specifications being used for general artifact management? The question that comes to mind after reading this thread, watching the last meeting recording, and discussing it with various parties is: what is the vision for OCI, OCI 2.0, and how does 1.1 fit into that vision? What are we building towards? If the answer is that general artifact management will be by convention within "container image" specifications, then that seems like shaky ground for organizations to base their enterprise tooling and plans around.

tianon
tianon previously approved these changes Apr 13, 2023
The artifact manifest does not confer any additional benefits beyond the
existing image manifest. Drop it for the sake of backward compatibility.

Remove references to artifact.md, regenerate graph.

Signed-off-by: Jon Johnson <[email protected]>
@jeffb4
Copy link

jeffb4 commented Apr 26, 2023

For historical purposes only, did the WG achieve 2/3 consensus on this in a meeting, in an email thread, otherwise?

@samuelkarp
Copy link
Member

Hi @jeffb4,

To clarify, the WG wasn't an authoritative body on any of the specs (the charter section 6. p. describes the scope of a WG). This project (the image-spec) is managed according to its governance by the maintainers.

Hopefully that helps!
Sam

@tianon
Copy link
Member

tianon commented May 15, 2023

I've been asked about this privately several times now, so as a maintainer (new to image-spec maintainer, but long-time OCI maintainer/contributor otherwise), I figured it was worth clarifying explicitly/publicly:

This does not remove the ability to store "artfiacts" in an OCI-compatible format or host "artifacts" on an OCI-compatible container image registry. This PR only removes support for the separate "artifact manifest" (which had not been in a GA release of the image-spec yet).

The end result of this was more explicitly codifying the "older" (and more widely compatible with tools and existing registries, especially when dealing with child objects and things that operate on the DAG such as garbage collection) method of storing and managing arbitrary artifacts via the existing image manifest type with a non-application/vnd.oci.image.config.v1+json config descriptor mediaType value and/or separate artifactType value, and the codification of an explicit "scratch" mediaType (application/vnd.oci.scratch.v1+json) and/or value ({}) for use cases which do not want or need to use some of the blobs that the image manifest otherwise requires (when maximizing compatibility across registries/tools, that's the config blob and at least one layer, both of which the scratch blob can be used for).

See also:

TLDR:

  1. the OCI image specification still very much supports arbitrary artifacts
  2. the format of those is largely compatible with most OCI image-spec 1.0 / distribution-spec 1.0 registries and tools today
  3. there is a new explicitly defined mediaType of application/vnd.oci.scratch.v1+json and value of {} for "stubbing" blobs that are not required by a given use case (but are otherwise required by the format/many tools)

sudo-bmitch pushed a commit to sudo-bmitch/distribution-spec that referenced this pull request Aug 18, 2023
This change corresponds with
opencontainers/image-spec#999
as we reconcile changes needed to allow referrers API (`subject` field
and `artifactType` field responses) to function smoothly on
image-manifest.

Personal take on this:
After ~~excesive~~ extensive deliberation, that while it would be nice
to have a clean-break to work with a generic manifest, we would be
setting ourselves and the whole ecosystem up for the compatibility
challenges similer to Docker schema 1 -> schema 2 -> OCI v1.

The biggest value for the community and ecosystem is to provide guidance
on how to use the available data structures and APIs to already
accomplish the same use-cases in the most broadly compatible way.

Signed-off-by: Vincent Batts <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.