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.

Sign up for GitHub

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

image-layout: describe an OPTIONAL index #438

Closed
wants to merge 1 commit into from
Closed

image-layout: describe an OPTIONAL index #438

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions image-layout.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ The image layout MUST also contain an `oci-layout` file:
- It MUST contain an `imageLayoutVersion` field
- The `imageLayoutVersion` value will align with the OCI Image Specification version at the time changes to the layout are made, and will pin a given version until changes to the layout are required
- It MAY include additional fields
- It MAY contain an `index` field ([described below](#index))

This is an example image layout:

Expand Down Expand Up @@ -129,4 +130,40 @@ $ cat ./blobs/sha256/e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7f
[tar stream]
```

## Index

Index is intended to be generated from the content in an image-layout after any updates have been made.
Copy link
Contributor

Choose a reason for hiding this comment

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

“any updates” → “any ref additions or removals”. You shouldn't need to update it after other changes (e.g. blob addition or removal, or changing a ref's descriptor payload).

Copy link
Member Author

Choose a reason for hiding this comment

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

Those are both are updates of one kind or another.

Copy link
Contributor

Choose a reason for hiding this comment

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

But there are updates that are not included in “ref additions or removals” (e.g. blob addition, or changing a ref's descriptor payload) which would not require index regeneration. I'm suggesting we make our regeneration suggestions as narrowly-scoped as possible, because conflicts around regenerating this index are one of the major pain-points with this approach.

Allows for mapping objects in the image-layout, and needing only to read from the `oci-layout` once to collect this index.

**index** *array of objects*, OPTIONAL
Copy link
Contributor

Choose a reason for hiding this comment

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

The ref list could get large, and I'd rather not overload the “do I understand how to parse this?“ oci-layout. Can we put this index in refs.json instead? You could just dive in with the array:

$ cat refs.json
["latest", "v1"]

Since the filename + the version from oci-layout sufficiently namespace it.

Copy link
Member Author

Choose a reason for hiding this comment

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

it has the potential for being long, no matter where it lives. And as the oci-layout is required file, it will already be stat'ed or fetched. Deferring to another file means another read, that could be a ENOENT or a 404. Plus this truncated list you mention does not allow for having tagged metadata.

Copy link
Contributor

Choose a reason for hiding this comment

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

With a separate file, a long index only impacts ref-listing. By putting it in oci-layout, a long index impacts any image-layout activity (because everyone should be checking to see “is this an image-layout version I understand” before trying to use the bits they need).

Plus this truncated list you mention does not allow for having tagged metadata.

If annotating refs is important, I'd rather have annotations added to the descriptor model. Do you have an example use case that shows that annotations are useful for refs but not for descriptors in general?

Copy link
Member Author

Choose a reason for hiding this comment

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

That still means that a read per ref.
The whole notion of an index will be for those sharing it in their own use-case being able to bubble up the important pieces of metadata per ref. Adding it to the descriptor might be fine as well, but as RPM, debian, and other repos have shown, there is a vast amount of metadata provided as early as possible. It is not my job to limit to these unknown users what information they're allowed to bubble up. Again, I am not just thinking through my own personal use-case of what is needed in my home directory of my personal machine.

Copy link
Contributor

Choose a reason for hiding this comment

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

Adding annotations to descriptors is not directly coupled to removing it from an index. It just means that folks who find the ref payload via refs/ will still have access to it (and can skip loading the potentially large index JSON). Once you have descriptor annotations, you can certainly cache them in the index as a separate performance optimization. But only supporting ref annotations via the index (and not via refs/) seems like a strange position.


Provides a single access to top-level information about the layout.
Copy link
Contributor

Choose a reason for hiding this comment

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

Four-space indents nside list entries where we'll be using sub-lists (opencontainers/runtime-spec#495)?

And for the wording, maybe just “Provides a list of available refs.”

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the same indentation used in the config.md and manifest.md for its field descriptions.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you want to punt for now, that's fine with me (I'm not a consumer of the compiled HTML or PDF output). But runtime-spec has had both opencontainers/runtime-spec#495 and opencontainers/runtime-spec#608 to avoid issues with two-space indents for sub-lists. It seems like we should be sticking to that pattern here for new additions, and eventually someone will be bothered enough by:

<ul>
<li><strong><code>mediaType</code></strong> <em>string</em></li>
</ul>
<p>This REQUIRED property contains the MIME type of the referenced content.</p>
<p>The OCI image specification defines <a href="media-types.md">several of its \
own MIME types</a> for resources defined in the specification.</p>

That they'll go through and fix the existing Markdown ;).


- Each object has the base properties of [descriptor][descriptors]
- OPTIONAL **annotations** field with the same constraints as [manifest `annotations`](manifest.md#image-manifest-property-descriptions)

### Index Example

```json
{
"imageLayoutVersion": "1.0.0",
"index": [
{
"mediaType": "application/vnd.oci.image.manifest.v1+json",
"digest": "sha256:0169d60159b9e6cf7aad602377ad6e3f551a7b0d01c51c8620cea5358f71546e",
"size": 426
},
{
"mediaType": "application/vnd.oci.image.manifest.v1+json",
"digest": "sha256:42f20fd615e5ab67f1b43c99f5f4a262d3e952ee6aa70b88f61207c24f2df2fb",
"size": 426,
"annotations": {
"org.waffles.with": "syrup,raspberries",
"io.flatpak.app.name": "org.gnome.gedit"
}
}
]
}
```

[descriptors]: ./descriptor.md